Beto A. 0 Denunciar post Postado Julho 13, 2011 Boa noite, amigos Estive aqui a brincar com POO e com PDO e fiz uma classe para inserção de produtos em uma tabela do banco usando o PDO Gostaria que vocês dessem uma olhada e me dissessem onde posso melhorar e o que deveria mudar, sou iniciante em OO e nunca usei o PDO, está é a primeira vez, por isso gostaria que me mostrassem se fiz algo errado, e me indicassem as melhores práticas, segue a classe: Product.Class.php <?php class Product { //$sku código produto private $sku; //$title titulo do produto private $title; //Construtor //function __construct(); /** * setSku * define a propriedade sku * * return $this->sku **/ public function setSku($str) { if(empty($str)){ $msg = 'O código Sku não pode ser vazio'; return $msg; } else { $this->sku = $str; } } /** * getSku * retorna o valor de sku * * @var $sku **/ public function getSku() { return $this->sku; } /** * setTitle * define a propriedade title * * return $this->title **/ public function setTitle($str) { if(empty($str)){ $msg = 'O title não pode ser vazio'; return $msg; } else { $this->title = $str; } } /** * getTitle * retorna o valor de title * * @var $sku **/ public function getTitle() { return $this->title; } public function insertProduct() { if((empty($this->title)) && (empty($this->sku))){ $msg = 'Os dados estão vazios'; return $msg; } else { try{ $dbh = new PDO("mysql:host=localhost;dbname=teste", "root", ""); } catch(PDOException $exception) { echo "Connection error: " . $exception->getMessage(); } $query = "INSERT INTO products VALUES ('', :sku, :title)"; $stmt = $dbh->prepare($query); $stmt->bindParam(':sku', $this->sku, PDO::PARAM_STR); $stmt->bindParam(':title', $this->title, PDO::PARAM_STR); $stmt->execute(); } } } Compartilhar este post Link para o post Compartilhar em outros sites
Guilherme Oderdenge 42 Denunciar post Postado Julho 13, 2011 Olá, Beto. Para a primeira vez mexendo com ambos, parece estar bem bacana. Olhei de cima e o que você pode alterar é o seguinte: As suas variáveis privadas, economizando linhas: private $sku, $title; Você pode definir as informações de conexão com o banco em um define: define('HOST','localhost'); define('USUARIO','root'); define('SENHA',''); define('DATABASE','database'); $dbh = new PDO("mysql:host=".HOST.";dbname=".DATABASE, USUARIO, SENHA); Você pode separar a conexão com o banco e chamá-la em uma espécie de "autoload" para deixar o arquivo mais flexível. Quando você for definir uma informação à uma variável em uma query PDO, você pode usar PDO::PARAM_INT // para números inteiros PDO::PARAM_STR // para strings Observações * Não vejo a necessidade de definir em uma variável a sua query. * Gostei do catch que você fez ali. No mais, tá bem legal seu script cara. Tá no caminho. :) Compartilhar este post Link para o post Compartilhar em outros sites
Leozitho 81 Denunciar post Postado Julho 13, 2011 Você deveria criar uma classe de conexão com o banco de dados. Não faz sentido você abrir a conexão com o banco de dados dentro do método insertProduct, ela já deveria estar aberta fora da classe ou então você poderia usar uma classe Singleton de conexão para verificar se já está aberta e se não estiver abrir. Outra coisa, você está usando o DSN com 'mysql' também dentro do método insertProduct: $dbh = new PDO("mysql:host=localhost;dbname=teste", "root", ""); Imagino que você tenha o costume de usar o PDO dessa forma em todos os seus métodos e classes. Mas imagine se um dia você tiver que mudar de MySQL para outro banco de dados, você teria que mudar cada método com conexão PDO de cada classe. Outra coisa que achei estranha é a forma como você está lidando com os erros: public function setSku($str) { if(empty($str)) { $msg = 'O código Sku não pode ser vazio'; return $msg; } else { $this->sku = $str; } } Acho que ficaria melhor se você retornasse um valor do tipo boolean (true ou false) para saber se o valor foi setado com sucesso ou não: public function setSku($str) { if (!empty($str)) { $this->sku = $str; return true; } return false; } Ou melhor ainda, trabalhar com exceptions: public function setSku($str) { if(empty($str)) { throw new Exception('O valor SKU não pode ser vazio.'); } $this->sku = $str; } No momento tenho essas dicas pra você dar uma melhorada aí. :) Compartilhar este post Link para o post Compartilhar em outros sites
João Batista Neto 448 Denunciar post Postado Julho 13, 2011 As suas variáveis privadas, economizando linhas: private $sku, $title; Eu não vejo muito sentido em fazer isso, na verdade, acho isso um erro. Qualquer modificação em um código que prejudique a leitura só deve ser feita quando a redução vá causar um incremento de desempenho que a justifique -- Não é o caso dessa modificação. Se outra pessoa for ler um código onde as propriedades estão organizadas dessa forma terá uma grande dificuldade em lê-lo. Você pode definir as informações de conexão com o banco em um define: define('HOST','localhost'); define('USUARIO','root'); define('SENHA',''); define('DATABASE','database'); Me dá arrepios só de ver um define em um código. Se quer utilizar constantes para definir essas informações (ignorando o fato de utilizar constantes no caso de dados de conexão é um grande erro), o ideal seria utilizar constantes de classe ou interface para fazê-lo; Afinal, o tópico é sobre orientação a objetos. Você pode separar a conexão com o banco e chamá-la em uma espécie de "autoload" para deixar o arquivo mais flexível. Isso sim foi uma dica interessante. Existem padrões específicos para isso, como Registry. O autoload é irrelevante, pode ser utilizado sim para facilitar, mas eu pessoalmente tenho evitado-o. * Não vejo a necessidade de definir em uma variável a sua query. Concordo. Tá no caminho. Concordo. Ou melhor ainda, trabalhar com exceptions: Definitivamente é melhor. Compartilhar este post Link para o post Compartilhar em outros sites
Andrey Knupp Vital 136 Denunciar post Postado Julho 13, 2011 João, se for conectar em um banco de dados só, eu acho que o melhor é usar Singleton .. Compartilhar este post Link para o post Compartilhar em outros sites
João Batista Neto 448 Denunciar post Postado Julho 13, 2011 João, se for conectar em um banco de dados só, eu acho que o melhor é usar Singleton .. Singleton jamais é melhor para banco de dados. Já ilustrei o problema em outros tópicos. Compartilhar este post Link para o post Compartilhar em outros sites
Beto A. 0 Denunciar post Postado Julho 13, 2011 Bom dia, caros... @Guilherme, sobre as variaveis como o João disse, eu também não acho legal fazer dessa forma, tendo em vista que prejudica a leitura do código e não nos da ganho nenhum em desempenho. @Leozitho eu imaginei que me diriam sobre isso, e na verdade até tentei criar uma classe de conexão, mas não consegui usando o PDO, você tem algum materia para me indicar onde possa ver isso? estou estudando usando o livro Dominando PHP e MySQL, Do Iniciante ao Profissional, mas lá não tem essa informação, se você puder me indicar um material ou exemplo ficarei muito grato. Sobre os erros, não me atentei que poderia lidar com eles usando exceptions, realmente fica bem melhor assim, vou mudar e fazer dessa forma, valeu a dica @João então não é interessante ou recomendado usar constantes para definir os dados de conexão? Seria interessante eu criar uma classe para me conectar ao banco, e estender ela nas minhas outras classes então? No mais, obrigado a todos pelas dicas, vou estudar os conselhos, posto os resultados que eu conseguir aqui. Compartilhar este post Link para o post Compartilhar em outros sites
João Batista Neto 448 Denunciar post Postado Julho 13, 2011 @João então não é interessante ou recomendado usar constantes para definir os dados de conexão? Imagine que amanhã você escreva uma aplicação na forma de produto, agora imagina que essa aplicação será colocada em servidores que seus clientes definirão. O que eles vão fazer ? Editar seu código e modificar as constantes ? E se houver uma migração de servidor ? Você editará seu código para modificar os dados de conexão ? Enfim, dados de conexão, assim como qualquer tipo de credencial para qualquer tipo de autenticação, é variável e não constante. Compartilhar este post Link para o post Compartilhar em outros sites
Bruno Augusto 417 Denunciar post Postado Julho 13, 2011 Não acho de todo errado usar constantes para informações de acesso ao banco, afinal elas são, de fato, constantes à aplicação. Uma vez preenchidas, seja manualmente, seja automaticamente, através de um instalador elas não irão mudar ao longo do desenvolvimento da classe. Claro que algo como: public function connect( $user = self::USER, $pwd = self::PWD, $db = self::DB, $host = self::HOSR ) { } Seria melhor e mais flexível já que, atrelado à uma interface, qualquer adaptador, de qualquer BD, que pode vir a ter informações de acesso diferentes, poderão ser corretamente configuradas usando-se credenciais diferentes das presentes nas constantes. Mas em sistema de menor porte, que jamais tenciona em usar mais de um banco de dados, não vejo como errado. Compartilhar este post Link para o post Compartilhar em outros sites
João Batista Neto 448 Denunciar post Postado Julho 14, 2011 Não acho de todo errado usar constantes para informações de acesso ao banco, afinal elas são, de fato, constantes à aplicação. Uma vez preenchidas, seja manualmente, seja automaticamente, através de um instalador elas não irão mudar ao longo do desenvolvimento da classe. :huh: Lamento dizer Bruno, mas essa sua afirmação me fez lembrar dessa tirinha: E acredite, minha reação foi exatamente: "POOTZ" Imagine que você tenha uma empresa chamada Invision e que você desenvolva um produto chamado Power Board. Agora, imagine que você coloque as credenciais de autenticação no banco como constantes no seu código: Já pensou que todos aqueles que adquirirem seu produto terão que editar seu código para ajustar as credenciais ? Sempre que você coloca credenciais de autenticação como constantes no código, você fará com que esse código não seja reutilizável, ou seja, você sempre precisará fazer Copy 'n' Paste, editar o código para poder aproveitar em outra aplicação. Compartilhar este post Link para o post Compartilhar em outros sites
Bruno Augusto 417 Denunciar post Postado Julho 14, 2011 Agora, imagine que você coloque as credenciais de autenticação no banco como constantes no seu código: Já pensou que todos aqueles que adquirirem seu produto terão que editar seu código para ajustar as credenciais ? Desculpa João, mas essa sua posição parece maluquice. :P Veja, ainda sob o exemplo do IP.Board. Você com certeza já viu o source dele e sabe onde ficam as informações de conexão, assim como outras tantas que nem deveriam ali estar. Trata-se do arquivo conf_global.php. Dentre tudo que ele possui, vemos: $INFO['sql_driver'] = 'mysql'; $INFO['sql_host'] = 'localhost'; $INFO['sql_database'] = 'database'; $INFO['sql_user'] = 'username'; $INFO['sql_pass'] = 'password'; $INFO['sql_tbl_prefix'] = 'ibf_'; Qual a diferença entre um array global e uma constante? A constante pelo menos dispensa uma palavra-chave global ou o array superglobal $GLOBALS (que eu acho pior). Ou desnecessariamente passar atribuir tais informações à um Registry para poder usar "Orientado a Objetos". Na primeira instalação, essas informações não existem e ao executar o instalador do IP.Board, elas são criadas, através das rotinas que invocam o arquivo install.php presente em admin/install/setup/sources/base. Na primeira instalação, o usuário não verá nada além de um wizard que o pegará pela mão e o guiará até o final do processo. Mas se uma migração de servidor for necessária e por algum motivo o usuário final não se lembrar das credenciais anteriormente usadas? Ao setar as novas no cPanel, poderá potencialmente usar outras diferentes. O que ele fará? Vai abrir o arquivo e editar manualmente as informações de acesso. Compartilhar este post Link para o post Compartilhar em outros sites
tesla 0 Denunciar post Postado Julho 14, 2011 Pessoal, vou pegar um embalo neste tópico para evitar criar outro. Tenho uma pequena dúvida: se eu tenho um sistema que trabalhará somente com MySQL por exemplo, eu levaria alguma vantagem em utilizar o PDO? Afinal, não vejo necessidade de abstrair nada, já que o sistema será desenvolvido somente para MySQL. Compartilhar este post Link para o post Compartilhar em outros sites
William Bruno 1501 Denunciar post Postado Julho 14, 2011 @tesla, eu pessoalmente, trabalho apenas com mysqli. Mas era bacana abrir outro tópico, pq isso pode gerar uma boa discussão, sobre "PDO é mais seguro", "mysqli é mais rápido".. Compartilhar este post Link para o post Compartilhar em outros sites
Beto A. 0 Denunciar post Postado Julho 14, 2011 Buenas galera... vejo que o tópico se tornou uma discussão saudavel e bacana, que bom. Quero mais uma vez agradecer a todos pelas dicas. Vamos lá Seguindo o conselho dos amigos, procurei na net pelo padrão Singleton, e uma classe construida assim para conexão ao banco, cheguei a esse tutorial: http://objota.com.br/web/php/classe-de-conexao-com-php-utilizando-pdo.html Na verdade, praticamente copiei a classe dele e vou estudar ela, apenas fiz uma pequena modificação para setar o retorno de erros como exceptions, vejam: <?php Class Conexao extends PDO { private static $instancia; public function Conexao($dsn, $username = "", $password = ""){ //Construtor do PDO parent::__construct($dsn, $username, $password); } public static function getInstance(){ //Se a instancia não existir eu faço uma if(!isset(self::$instancia)){ try { self::$instancia = new Conexao("mysql:host=localhost;dbname=pdo", "root", ""); self::$instancia->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); } catch (Exception $e) { echo 'Houve um erro ao conectar: ' . $e->getMessage(); } } } } Essa classe está digna de ser usada? e, como aplicar ela a meu exemplo anterior? Compartilhar este post Link para o post Compartilhar em outros sites
Leozitho 81 Denunciar post Postado Julho 14, 2011 Então Beto, eu particularmente quando comecei a estudar e trabalhar com POO já usei a classe de conexão dessa forma usando o Singleton e não vi problema nenhum. Inclusive é o que mais se recomenda na internet, a maioria dos artigos, como o que você encontrou, ensinam a trabalhar dessa forma. Agora só parei de usar dessa forma porque estou usando mais o Zend Framework em meus projetos e não mais o PHP "puro" digamos assim. Agora se é certo ou não, vai do ponto de vista de cada um. Para o nosso amigo João Batista, por exemplo, usar Singleton com banco de dados é um "crime", como ele costuma dizer. Para ele o mais apropriado seria usar o pattern Registry. Mas como ele mesmo disse recentemente em outro tópico, ele já está com quase 30 anos de idade e começou a programar se eu não me engano com 14 anos, ou seja, é um programador com mais de 10 anos de experiência. Além do mais, pelo o que já andei vendo ele programa também na linguagem Java, que é 100% orientada a objetos e exige um conhecimento muito mais aprofundado em patterns e tudo mais. O PHP até recentemente tinha pouquíssimos (quase nenhum) recursos para se trabalhar com POO, e a maioria das aplicações desenvolvidas com ele não são orientadas a objetos. Inclusive um dos sistemas web mais usados por sites e blogs no mundo inteiro, o Wordpress, utiliza constantes do PHP para guardar as credenciais de configuração, coisa que o nosso amigo João falou que é errado. Ao meu ver, os desenvolvedores da Automattic (empresa que desenvolve o Wordpress e outros sistemas como o Gravatar) não são menos profissionais porque mantem a aplicação estruturada dessa forma. Sinceramente eu acredito que não existe nada 100% certo ou 100% errado, depende do ponto de vista de cada um. :) Compartilhar este post Link para o post Compartilhar em outros sites
Beto A. 0 Denunciar post Postado Julho 14, 2011 Boa noite, amigos @leozitho, obrigado pelas dicas amigo! é muito bom ouvir e poder contar com pessoas mais experientes que nós. Vou me aprofundar mais nos estudos disso, o livro que uso para estudos de PHP não falava nada sobre usar o Pattern, mas vou procurar material na net... agora vou tentar aplicar essa classe de conexão com a minha classe de estudo Products, posto o resultado aqui para analise depois. Abraço Compartilhar este post Link para o post Compartilhar em outros sites
Beto A. 0 Denunciar post Postado Julho 15, 2011 Boa tarde, galera Ao tentar usar a classe de conexão que postei a cima, com uma classe apra inserção de clientes, tive um erro... Segue a classe: <?php include('Connect.class.php'); class Cliente { //Nome do cliente private $nome; //Email private $email; //Telefone private $telefone; //Instancia do banco private $db = Conexao::getInstance; public function setNome($str){ $this->nome = $str; } public function setEmail($str){ $this->email = $str; } public function setTelefone($str){ $this->telefone = $str; } public function adicionaCliente(){ try { $sql = "INSERT INTO clientes VALUES ('', :nome, :telefone, :email)"; $query = $db->prepare($sql); $query->bindParam(':nome', $this->nome, PDO::PARAM_STR); $query->bindParam(':telefone', $this->telefone, PDO::PARAM_STR); $query->bindParam(':email', $this->email, PDO::PARAM_STR); $query->execute(); } catch(PDOException $e) { echo 'Houve um erro: ' . $e->getMessage(); } } } E o erro: Alguem tem alguma ideia do que pode ser? Compartilhar este post Link para o post Compartilhar em outros sites
Bruno Augusto 417 Denunciar post Postado Julho 15, 2011 Aqui ó: private $db = Conexao::getInstance; Você está chamando uma suposta constante chamada getInstance a partir do escopo da classe Conexao. Como ela não existe, gerou erro. O certo é colocar com parênteses no final, já que getInstance é ummétodo da classe Conexão. Porém, se você simplesmente colocar esses parênteses, você vai ter, se não me falha a memória, um Syntax Error. O certo seria essa declaração estar no __construct() da classe Cliente, sendo esta proprieda ($db) sendo acessada através da pseudo-variável $this: public function __construct() { $this -> db = Conexao::getInstance(); } Compartilhar este post Link para o post Compartilhar em outros sites
Beto A. 0 Denunciar post Postado Julho 15, 2011 Salve Bruno, boa noite... testei essa modificação que você me instruiu a classe ficou dessa forma: <?php include('Conexao.class.php'); class Cliente { //Nome do cliente private $nome; //Email private $email; //Telefone private $telefone; //Instancia do banco private $db; public function __construct(){ $this->db = Conexao::getInstance(); } public function setNome($str){ $this->nome = $str; } public function setEmail($str){ $this->email = $str; } public function setTelefone($str){ $this->telefone = $str; } public function adicionaCliente(){ try { $sql = "INSERT INTO clientes VALUES ('', :nome, :telefone, :email)"; $query = $db->prepare($sql); $query->bindParam(':nome', $this->nome, PDO::PARAM_STR); $query->bindParam(':telefone', $this->telefone, PDO::PARAM_STR); $query->bindParam(':email', $this->email, PDO::PARAM_STR); $query->execute(); } catch(PDOException $e) { echo 'Houve um erro: ' . $e->getMessage(); } } } Mas agora gerou esse erro: Fatal error: Call to a member function prepare() on a non-object in C:\wamp\www\Estudos\Dominando PHP e MySQL\PDO\Cliente.class.php on line 48 Compartilhar este post Link para o post Compartilhar em outros sites
Guilherme Oderdenge 42 Denunciar post Postado Julho 15, 2011 Olá, Beto. Na sua query de inserção você utilizou $db->, sendo que o certo seria $this->db. Abraço! Compartilhar este post Link para o post Compartilhar em outros sites