felipe_zmm 0 Denunciar post Postado Abril 10, 2013 Pessoal, desculpem a imensa demora na resposta. Fiquei fora uns dias e não tive como dar andamento aos estudos práticos e acompanhar o tópico. Observe que dos 3 métodos existentes, apenas 1 utiliza a instância de PDO, logo, ela não é tão essencial assim, logo, você não deveria impor que para instanciar ControleLogin você precisa de um objeto PDO. A única dependência forte é a classe de sessão. Eu tinha feito dessa forma que você sugeriu (post #31) e me recomendaram não utilizar mais de três parâmetros em um método e a passar o PDO no construtor. Mas a classe tem outros problemas. Exemplo: você restringe o uso de sha1 como método de critografia, mas e se quiser alterar? Se por exemplo, existem 2 tipos de autenticação diferente no seu sistema, um usa md5 e o outro sha1, como você faria? Eu deveria então criar uma classe de criptografia ? [inline]ControleLogin[/inline] depende de [inline]User[/inline] para obter as credenciais e torná-lo logado. Precisa, também, de uma [inline]Session[/inline] para guardar o usuário logado entre as requisições. Por fim, sua tarefa apenas será executada se as credenciais passarem por um [inline]Validator[/inline]. Essa abordagem de passar um objeto User para um Validator me causa uma certa confusão. A minha classe Usuario contém propriedades que não dizem respeito apenas sobre o login em si, porque eu a utilizarei também para fazer inserções e selects no banco de dados. class Usuario { private $id; private $nome; private $sobrenome; private $email; private $login; private $senha; private $status; No caso da autenticação as únicas propriedades utilizadas seriam login e senha. Quero dizer, é correto passar um objeto que tem sete propriedades sendo que apenas duas serão úteis no contexto ? Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 10, 2013 Eu tinha feito dessa forma que você sugeriu (post #31) e me recomendaram não utilizar mais de três parâmetros em um método e a passar o PDO no construtor.Eu tinha feito dessa forma que você sugeriu (post #31) e me recomendaram não utilizar mais de três parâmetros em um método e a passar o PDO no construtor.Um método deve ter tantos parâmetros quanto necessitar, quem sugeriu isso não sabe o que fala :closedeyes: . Eu deveria então criar uma classe de criptografia ? Mais ou menos isso. Lhe apresento Strategy: :seta: http://sourcemaking.com/design_patterns/strategy Apesar de o pessoal do mundo Java não gostar muito do que vou sugerir, isso é perfeitamente válido para PHP. Ficaria ainda melhor usando PHP 5.3+, no qual você pode usar closures. class ControleLogin { private $sessao; private $passwordEncrypter; public function __construct(Sessao $sessao, $encrypter) { if(!is_callable($encrypter)) { throw new Exception('Invalid password encrypt algorithm'); } $this->sessao = $sessao; $this->passwordEncrypter = $encrypter; } // ... // ... private function gerarHash($senha) { return call_user_func($this->passwordEncrypter, $senha); } } // PHP 5.0+ $cLogin = new ControleLogin(Sessao::instancia(), 'sha1'); //ou PHP 5.3+ $cLogin = new ControleLogin(Sessao::instancia(), function($str) { // Faça sua própria criptografia }); //ou PHP 5.3+ $cLogin = new ControleLogin(Sessao::instancia(), function($str) { return md5($str); }); A única restrição é que a estratégia de criptografia deve conseguir fazer isso com base em apenas um parâmetro (a maioria faz exatamente isso). Compartilhar este post Link para o post Compartilhar em outros sites
Vinicius Rangel 208 Denunciar post Postado Abril 10, 2013 ControleLogin depende de User para obter as credenciais e torná-lo logado. Precisa, também, de uma Session para guardar o usuário logado entre as requisições. Por fim, sua tarefa apenas será executada se as credenciais passarem por um Validator. Veja a perfeita harmonia e sintonia entre esses objetos, cada um tem sua função dentro da aplicação e ela não faz nada mais além do que isso. Veja que a frase foi escrita em um fluxo no qual o seu usuário passa. Cria instancia do objeto pessoa. Então agora nós temos uma pessoa no nosso sistema, mas uma pessoa sem credenciais não faz nada no meu site. Ok, vamos arrumar credenciais pra ela. Joguei o objeto pessoa dentro de um objeto de validação. Dentro do objeto de validação caso ele tenha as devidas credenciais você pode liberar a passagem dele (Criar uma sessão). Depois o que você fará nada mais é que em todo lugar que ele for perguntar para ele. Ou você existe né? vc ta ai? Mesmo que pareça redundante você precisa confirmar essa informação. Onde eu quero chegar com isso? O seu programa pode ser lida como se fosse uma história o que torna teu entendimento muito melhor. Orientado a objeto é tratar suas classes como "objetos Reais". Um método deve ter tantos parâmetros quanto necessitar Concordo mas tem que ter uma boa avaliação do que é necessário. As vezes a galera quer jogar todos parametros dentro do construtor sendo que na vdd é uma propriedade do objeto e não do método (eu fazia muito isso no começo). Compartilhar este post Link para o post Compartilhar em outros sites
Gabriel Heming 766 Denunciar post Postado Abril 10, 2013 Um método deve ter tantos parâmetros quanto necessitar, quem sugeriu isso não sabe o que fala :closedeyes: . Eu não queria sair do escopo do tópico, mas achei que seria interessante comentar. Robert "Uncle Bob" Martin, o qual não é qualquer "programador", é quem sugere isso. Para quem quiser ler o trecho que ele fala sobre isso: The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justicationand then shouldnt be used anyway. [...] When a function seems to need more than two or three arguments, it is likely that some of those arguments ought to be wrapped into a class of their own. Como os exemplos são em Java, não há muita distinção entre função e método. Ele fala function, pelo tipo de abordagem dos conteúdos. Quando ele entra em conceitos de classes e objetos, como métodos, ele apenas expande para o conceito de classe e objeto, pois os outros conceitos como parâmetros e retorno (por exemplo), já foram abordados. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 10, 2013 E nisso vemos um monte de gente transformando diferentes parâmetros em arrays, só pra diminuir o número de parâmetros. Como eu já disse aqui, esses "mantras" servem muito mais pra assustar o programador newbie pra ele não fazer besteira do que para uso prático. Claro que você não vai passar 40 parâmetros pra um método, mas restringir a um número X é bobagem. Você precisa ajustar isso as suas necessidades, sejam 3, 4, 5 parâmetros, a não ser que você queira cair em casos como o do nosso colega ali que precisava instanciar um objeto PDO para usar uma função que não necessita dessa instância. Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 10, 2013 Mas Henrique, pense o seguinte, essa classe vai ser injetada em um método. Esse método para consumir a classe vai precisar instanciar a PDO e etc. Configurações são parte do estado da classe, não de parâmetros, programação orientada a objetos não é programação funcional. Quanto ao Singleton, persistindo minha opinião, ele é inútil, desnecessário e vai contra a orientação a objetos. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 10, 2013 Não consigo ver um objeto PDO como parte do estado dessa classe. Ela serve basicamente para fornecer/verificar o acesso de um usuário ao sistema. Quais são os casos de uso dessa clase? Autenticar o usuário Fazer logoff com o usuário já autenticado Pra fazer essas duas operações, o que eu preciso?Hmmmm, um manipulador de sessões, pois é com base nele que eu autentico o usuário, e é daí que eu tenho que remover o usuário ao fazer logoff. Isso me indica que TODO objeto ControleLogin precisa de um controlador de sessão pra poder realizar QUALQUER operação, ou seja, essa deveria ser uma dependência no construtor. Agora o que eu preciso pro caso de uso 1?Preciso de um usuário e uma senha pra fazer a autenticação, um possível método de criptografia que eu vou utilizar. Ah, claro, preciso saber como vou fazer a validação dos dados. Será que devo fazer de alguma dessas dependências uma propriedade da clase?A resposta depende muito. Várias coisas a considerar: Vou usar alguma dessas propriedades em outro método? O funcionamento de partes da classe depende dessa propriedade? Propriedades de classe, geralmente, dizem respeito ao seu estado. Alguma propriedade compõe o estado da classe? (aqui vale lembrar que algumas classes, apesar de possuírem atributos, são stateless). No caso de objetos, instanciar um objeto só pra satisfazer essa dependência não será um overhead? (nesse caso, sim) Segundo o meu entendimento, esta última questão mata a charada: PDO não é uma dependência de classe. Quanto ao Singleton, persistindo minha opinião, ele é inútil, desnecessário e vai contra a orientação a objetos.Ok, você já disse isso 400 vezes. É sua opinião, beleza, mas ela está errada, e eu já disse porque. ______________________________________________________________________________________ Quanto à classe ControleLogin, ainda vejo problemas. Imagine que você muda o mecanismo de verificação na tabela, que, por exemplo, as senhas agora passarão a ter um salt adicionado para aumentar a segurança. O que você faz? Vai lá e altera na classe ControleLogin o método de validação? Que tal algo assim? <?php class ControleLogin { private $manipuladorSessao; public function __construct(Sessao $manip) { $this->manipuladorSessao = $manip; } public function autenticar(ValidadorLogin $val, $usuario, $senha) { try { $val->execute($usuario, $senha); $this->manipuladorSessao->colocar('usuario', $usuario); $this->manipuladorSessao->colocar('validado', 1); } catch(Exception e) { throw new AuthException('Erro de autenticação. Causa: ' . $e->getMessage()); } } public function sair() { // Suponha que permite remover vários de uma vez passando um array $this->manipuladorSessao->remover(array('usuario', 'validado')); } } interface ValidadorLogin { /** * Executa a validação. * * @return void * @throws Exception se os dados não forem válidos */ public function executar(); /** * Estabelece o método de criptografia que o validador utilizará. * * @param string|Closure $algoritmo : o algoritmo de criptografia. * Pode ser o nome de uma função, uma closure ou um objeto * que implemente o método mágico __invoke(). */ public estabelecerCriptografia($algoritmo); /** * Obtém o método de criptografia que o validador utiliza. * * @return string|Closure */ public obterCriptografia(); } class PdoValidadorLogin implements ValidadorLogin { private $algoritmoCriptografia; private $instanciaPdo; public function __construct(PDO $pdo, $alg = null) { $this->instanciaPdo = $pdo; if($alg !== null) { $this->estabelecerCriptografia($alg); } } public function obterCriptografia() { return $this->algoritmoCriptografia; } public function estabelecerCriptografia($alg) { if(is_callable($alg)) { $this->algoritmoCriptografia = $alg; } else { throw new Exception('Algoritmo de criptografia inválido'); } } public function executar($usuario, $senha) { $senha = gerarHash($senha); $qry = "SELECT COUNT(id) FROM {$tabela} " . "WHERE login = :login AND senha = :senha AND status = '1'"; $stmt = $this->instanciaPdo->prepare($qry); $stmt->bindParam(':login', $usuario); $stmt->bindParam(':senha', $senha); $stmt->execute(); if($stmt->fetchColumn() != 1) { throw new Exception('Credenciais inválidas'); } } public function gerarHash($senha) { if($this->algoritmoCriptografia !== null) { return call_user_func($this->algoritmoCriptografia, $senha); } return $senha; } } class ArquivoSenhasValidadorLogin implements ValidadorLogin { private $filename; public function __construct($filename) { $this->filename = $filename; } // ... continua } Com esse exemplo, ControleLogin é reutilizável, você pode implementar apenas validadores e nunca mais mexer nessa classe.Talvez agora fique mais claro porque ValidadorLogin deve ser um parâmetro do método autenticar, não uma propriedade da classe: você precisa de um validador só pra autenticar o usuário, mas não para desconectá-lo. Compartilhar este post Link para o post Compartilhar em outros sites
felipe_zmm 0 Denunciar post Postado Abril 10, 2013 Que tal algo assim? <?php class ControleLogin { private $manipuladorSessao; public function __construct(Sessao $manip) { $this->manipuladorSessao = $manip; } public function autenticar(ValidadorLogin $val, $usuario, $senha) { try { $val->execute($usuario, $senha); $this->manipuladorSessao->colocar('usuario', $usuario); $this->manipuladorSessao->colocar('validado', 1); } catch(Exception e) { throw new AuthException('Erro de autenticação. Causa: ' . $e->getMessage()); } } public function sair() { // Suponha que permite remover vários de uma vez passando um array $this->manipuladorSessao->remover(array('usuario', 'validado')); } } interface ValidadorLogin { /** * Retorna o nome do usuário autenticado. * Se execute ainda não tiver sido executado, * fará a chamada desse método. * * @return string */ public function obterUsuario(); /** * Executa a validação. * * @return void * @throws Exception se os dados não forem válidos */ public function executar(); /** * Estabelece o método de criptografia que o validador utilizará. * * @param string|Closure $algoritmo : o algoritmo de criptografia. * Pode ser o nome de uma função, uma closure ou um objeto * que implemente o método mágico __invoke(). */ public estabelecerCriptografia($algoritmo); /** * Obtém o método de criptografia que o validador utiliza. * * @return string|Closure */ public obterCriptografia(); } class PdoValidadorLogin implements ValidadorLogin { private $algoritmoCriptografia; private $instanciaPdo; public function __construct(PDO $pdo, $alg = null) { $this->instanciaPdo = $pdo; if($alg !== null) { $this->estabelecerCriptografia($alg); } } public function obterCriptografia() { return $this->algoritmoCriptografia; } public function estabelecerCriptografia($alg) { if(is_callable($alg)) { $this->algoritmoCriptografia = $alg; } else { throw new Exception('Algoritmo de criptografia inválido'); } } public function executar($usuario, $senha) { $senha = gerarHash($senha); $qry = "SELECT COUNT(id) FROM {$tabela} " . "WHERE login = :login AND senha = :senha AND status = '1'"; $stmt = $this->instanciaPdo->prepare($qry); $stmt->bindParam(':login', $usuario); $stmt->bindParam(':senha', $senha); $stmt->execute(); if($stmt->fetchColumn() != 1) { throw new Exception('Credenciais inválidas'); } } public function gerarHash($senha) { if($this->algoritmoCriptografia !== null) { return call_user_func($this->algoritmoCriptografia, $senha); } return $senha; } } class ArquivoSenhasValidadorLogin implements ValidadorLogin { private $filename; public function __construct($filename) { $this->filename = $filename; } // ... continua } Com esse exemplo, ControleLogin é reutilizável, você pode implementar apenas validadores e nunca mais mexer nessa classe.Talvez agora fique mais claro porque ValidadorLogin deve ser um parâmetro do método autenticar, não uma propriedade da classe: você precisa de um validador só pra autenticar o usuário, mas não para desconectá-lo. Gostei da idéia. Só não entendi bem o propósito do método obterUsuario que não foi utilizado depois. Vou implementar algo nesse sentido e volto a postar. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 11, 2013 Ops, é que faltou tirar esse método, vou atualizar aqui... Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 11, 2013 Sim, eu não li todos os tópicos e não sei o andamento todo. Mas o ponto de vista é: estado que eu digo é uma propriedade, passada por construtor, caso contrário, se injetasse um objeto dessa clase em alguma outra, por exemplo, para usar qualquer método relacionado ao banco de dados teria que instanciar a PDO, e etc. como por exemplo: <?php class Executor { private $c; public function __construct(ControleLogin $c) { $this->c = $c; } public function fazerAlgo() { // treta.. $this->c->insert(new PDO); } } Mas isso está relacionado também ao problema da responsabilidade única, pois deve ter uma classe só para fazer as ações do banco. -------- Quanto ao Singleton, eu quero sinceramente acabar com essa discussão, mas até agora nada foi apresentado que justifica o uso de singleton. Vou deixar alguns links sobre esse padrão: http://stackoverflow.com/a/138012 http://c2.com/cgi/wiki?SingletonsAreEvil https://sites.google.com/site/steveyegge2/singleton-considered-stupid http://tech.puredanger.com/2007/07/03/pattern-hate-singleton/ http://googletesting.blogspot.com.br/2008/08/by-miko-hevery-so-you-join-new-project.html http://googletesting.blogspot.com.br/2008/08/root-cause-of-singletons.html http://googletesting.blogspot.com.br/2008/08/where-have-all-singletons-gone.html Tem muito, mas muito mesmo material sobre criticismo de Singleton, basta procurar e realmente enxergar os "roncos" do Singleton. Compartilhar este post Link para o post Compartilhar em outros sites
felipe_zmm 0 Denunciar post Postado Abril 13, 2013 Pessoal, implementei as classes de login seguindo as sugestões que me deram aqui. Vejam como ficou e se possível me deixem um feedback sobre as classes. class ControleLogin { private $sessao; public function __construct(Sessao $sessao) { $this->sessao = $sessao; } public function autenticarLogin(Autenticador $autLogin, Usuario $usuario) { try { $autLogin->autenticar($usuario->getLogin(), $usuario->getSenha()); $this->sessao->setarValor('usuario', $usuario); $this->sessao->setarValor('autenticado', 1); } catch (Exception $e) { throw new Exception($e->getMessage()); } } public function logout() { $this->sessao->destruirSessao(); } public function usuarioLogado() { return $this->sessao->valorSetado('autenticado'); } } interface Autenticador { public function autenticar($login, $senha); public function criptografar($senha); } class AutenticadorPdo implements Autenticador { private $pdo; private $cripto; public function __construct(PDO $pdo, $cripto = null) { $this->pdo = $pdo; if ($cripto != null) { $this->cripto = $cripto; } } public function setCripto($cripto) { if (!is_callable($cripto)) { throw new Exception('Criptografia inválida!'); } $this->cripto = $cripto; } public function getCripto() { return $this->cripto; } public function autenticar($login, $senha) { $qry = "SELECT COUNT(id) FROM usuarios " . "WHERE login = :login AND senha = :senha AND status = '1'"; $stmt = $this->pdo->prepare($qry); $stmt->bindParam(':login', $login); $stmt->bindParam(':senha', $this->criptografar(trim($senha))); $stmt->execute(); if ($stmt->fetchColumn() != 1) { throw new Exception('Usuário ou senha inválidos!'); } } public function criptografar($senha) { if ($this->cripto != null) { return call_user_func($this->cripto, $senha); } return sha1($senha); } } class AutenticadorTxt implements Autenticador { private $nomeArquivo; private $cripto; private $fp; public function __construct($arquivo, $cripto = null) { $this->nomeArquivo = $arquivo; if ($cripto != null) { $this->cripto = $cripto; } } public function setCripto($cripto) { if (!is_callable($cripto)) { throw new Exception('Criptografia inválida!'); } $this->cripto = $cripto; } public function getCripto() { return $this->cripto; } public function autenticar($login, $senha) { $senha = $this->criptografar($senha); $this->abrirArquivo(); while (!feof($this->fp)) { $arr = explode(', ', fgets($this->fp, 1024)); if (($login == $arr[0]) && ($senha == trim($arr[1]))) { $this->fecharArquivo(); return; } } $this->fecharArquivo(); throw new Exception('Usuário ou senha inválidos!'); } public function criptografar($senha) { if (is_callable($this->cripto)) { return call_user_func($this->cripto, $senha); } return sha1($senha); } private function abrirArquivo() { $this->fp = fopen($this->nomeArquivo, 'r'); } private function fecharArquivo() { fclose($this->fp); } } E utilizando isso com PDO ou arquivo texto: function __autoload($classe) { require_once("classes/{$classe}.class.php"); } $usuario = new Usuario(); $usuario->setLogin($_POST['usuario']); $usuario->setSenha($_POST['senha']); $pdo = new PDO('mysql:host=localhost;dbname=loja', 'root', ''); try { $cLogin = new ControleLogin(new Sessao(true)); $cLogin->autenticarLogin(new AutenticadorPdo($pdo), $usuario); //$cLogin->autenticarLogin(new AutenticadorTxt('login.txt'), $usuario); header("Location: /loja/home_admin.php"); } catch (Exception $e) { echo $e->getMessage(); } Bom, está tudo funcionando e procurei aplicar as dicas que o pessoal aqui me passou. Como o Henrique sugeriu eu utilizei uma interface Autenticador. Eu vejo bastante as pessoas utilizarem interface, mas nesse caso não seria melhor utilizar uma classe abstrata ? Porque os métodos setCripto, getCripto e criptografar seriam iguais e o que muda mesmo é o método autenticar. Enfim, mais uma vez obrigado a todos pela atenção que têm me dado. Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 13, 2013 Vamos lá, finalmente arrumei tempo para o tópico :) . Em relação à sua pergunta, eu não apagaria a interface. Criaria uma classe abstrata e nessa classe não implementaria nenhuma interface, apenas teria os métodos setCripto e getCripto; removeria esses métodos das classes de autenticadores e essas classes estenderiam a classe abstrata, mantendo a interface e removendo a duplicação. Bem, eu encontrei redundância no código, veja: No método autenticarLogin da classe ControleLogin você usa try/catch desnecessariamente. Quanto ao Strategy, eu sinceramente prefiro criar classes do que usar funções anônimas, esse problema você vê bem quando é necessário o if no método criptografar, dá um ar de mal cheiro. Minha implementação ficaria assim: Na classe de ControleLogin eu apenas removeria o try/catch desnecessário: class ControleLogin { private $sessao; public function __construct(Sessao $sessao) { $this->sessao = $sessao; } public function autenticarLogin(Autenticador $autLogin, Usuario $usuario) { $autLogin->autenticar($usuario->getLogin(), $usuario->getSenha()); $this->sessao->setarValor('usuario', $usuario); $this->sessao->setarValor('autenticado', 1); } public function logout() { $this->sessao->destruirSessao(); } public function usuarioLogado() { return $this->sessao->valorSetado('autenticado'); } } E agora seria a maior mudança: seus autenticadores possuem mais do que uma responsabilidade, em outras palavras violam o SRP basicamente pois além de autenticar, eles fazem a criptografia, e aí você viu que há uma repetição de código. Eu faria assim: - Primeiro criaria uma interface para os encriptadores: interface Encriptador { public function criptografar($senha); } E agora você pode criar quais encriptadores você quiser: class MD5 implements Encriptador { public function criptografar($senha) { return md5($senha); } } class SHA1 implements Encriptador { public function criptografar($senha) { return sha1($senha); } } class SHA256 implements Encriptador { public function criptografar($senha) { return hash('sha256', $senha); } } // crie quantos e quais quiser... E faria a mudança nos autenticadores, começando pela interface: interface Autenticador { public function autenticar($login, $senha); } E utilizaria agora o que criei antes, os encriptadores: class AutenticadorPdo implements Autenticador { private $pdo; private $encriptador; public function __construct(PDO $pdo, Encriptador $encriptador) { $this->pdo = $pdo; $this->encriptador = $encriptador; } public function autenticar($login, $senha) { $qry = "SELECT COUNT(id) FROM usuarios " . "WHERE login = :login AND senha = :senha AND status = '1'"; $stmt = $this->pdo->prepare($qry); $stmt->bindParam(':login', $login); $stmt->bindParam(':senha', $this->encriptador->criptografar(trim($senha))); $stmt->execute(); if ($stmt->fetchColumn() != 1) { throw new Exception('Usuário ou senha inválidos!'); } } } class AutenticadorTxt implements Autenticador { private $nomeArquivo; private $encriptador; private $fp; public function __construct($arquivo, Encriptador $encriptador) { $this->nomeArquivo = $arquivo; $this->encriptador = $encriptador; } public function autenticar($login, $senha) { $senha = $this->encriptador->criptografar($senha); $this->abrirArquivo(); while (!feof($this->fp)) { $arr = explode(', ', fgets($this->fp, 1024)); if (($login == $arr[0]) && ($senha == trim($arr[1]))) { $this->fecharArquivo(); return; } } $this->fecharArquivo(); throw new Exception('Usuário ou senha inválidos!'); } private function abrirArquivo() { $this->fp = fopen($this->nomeArquivo, 'r'); } private function fecharArquivo() { fclose($this->fp); } } Usando: function __autoload($classe) { require_once("classes/{$classe}.class.php"); } $usuario = new Usuario(); $usuario->setLogin($_POST['usuario']); $usuario->setSenha($_POST['senha']); $pdo = new PDO('mysql:host=localhost;dbname=loja', 'root', ''); // instancie seu encriptador $encriptador = new SHA1; try { $cLogin = new ControleLogin(new Sessao(true)); $cLogin->autenticarLogin(new AutenticadorPdo($pdo, $encriptador), $usuario); //$cLogin->autenticarLogin(new AutenticadorTxt('login.txt', $encriptador), $usuario); header("Location: /loja/home_admin.php"); } catch (Exception $e) { echo $e->getMessage(); } Perceba que a repetição foi removida e agora os encriptadores são os caras que fazem a criptografia e os autenticadores apenas autenticam, seguindo a responsabilidade única. :joia: Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 13, 2013 Bem, eu encontrei redundância no código, veja: No método autenticarLogin da classe ControleLogin você usa try/catch desnecessariamente.Muito bem observado, não faz sentido pegar uma exceção para lançá-la logo em seguida. Quanto ao Strategy, eu sinceramente prefiro criar classes do que usar funções anônimas, esse problema você vê bem quando é necessário o if no método criptografar, dá um ar de mal cheiro.No PHP 5.4+ esse if é desnecessário: :seta: http://www.php.net/manual/en/language.types.callable.php Eu acho overhead demais criar classes e objetos somente para chamar uma função built-in. PHP não é Java. Até mesmo o Java estuda uma maneira de fornecer suporte a closures como faz atualmente o PHP. O pseudo-tipo [inline]callable[/inline] engloba 3 coisas: Built-in functions e user-defined functions/methods Closures Classes que implementam o método abstrato [inline]__invoke()[/inline] Com isso, você não precisa criar classes só para chamar uma função como md5, sha1, etc, mas você também pode definir suas próprias classes de criptografia, tomando apenas o cuidado de implementar [inline]__invoke[/inline]. Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 13, 2013 Certo, concordo com o if.Em relação as minhas modificações, não é um overhead, é uma forma de reduzir a duplicação de código (que existia) e torná-lo mais limpo, além de que antes o código estava violando o SRP. E ainda mais: fazendo a coisa mais linda do mundo: tipos e mantendo a API mais sólida e coesa.callable nesse caso nada mais é que uma obsessão por primitivos (apesar de que callable não é um primitivo), esse tipo de interface não é algo interessante de se ter. E dizer que 'isso é coisa de Java' não é um argumento, mas sim uma falácia. Se desejar uma interface para ser 'amigável à preguiça', basta criar uma Façade. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 14, 2013 Em relação as minhas modificações, não é um overhead, é uma forma de reduzir a duplicação de código (que existia) e torná-lo mais limpo, além de que antes o código estava violando o SRP. E ainda mais: fazendo a coisa mais linda do mundo: tipos e mantendo a API mais sólida e coesa. Qual código viola o SRP? O do post 87 ou do 91. callable nesse caso nada mais é que uma obsessão por primitivos (apesar de que callable não é um primitivo), esse tipo de interface não é algo interessante de se ter. E dizer que 'isso é coisa de Java' não é um argumento, mas sim uma falácia. Java possui sua própria gambiarra pra nos pertimitir utilizar callbacks: button.addActionListener(new ActionListener(ActionEvent e) { public void actionPerformed (ActionEvent e) { // Create Frame 2 JFrame frame2 = new JFrame("Frame 2"); frame2.setSize(200,200); frame2.setVisible(true); frame2.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); JLabel label = new JLabel("Clicked!"); JPanel panel2 = new JPanel(); // First add to frame2 the panel just create frame2.add(panel2); // Add to panel the label panel2.add(label); } } }); Realmente precisamos de tudo isso? Como fazemos a mesma coisa com Javascript: button.addEventListener('click', function(event){ // ... }); Como uma linguagem dinâmica, o PHP se aproxima muito mais do Javascript do que do Java neste ponto. Do mesmo modo que muita gente se recusa a utilizar traits por serem "reuse by copy & paste", muita gente não gosta de callbacks, muita gente quer que o PHP seja fortemente tipado. Recursos novos não são adicionados a uma linguagem por nada. Se está lá, é porque tem um uso. Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 14, 2013 - Do post 91; os autenticadores além de autenticar, geram a criptografia e existia repetição de código. --- Quanto as funções anônimas, elas são excelentes e possuem muitos usos, mas para Strategy não é o ideal. Como você mesmo mostrou, elas tem uso para EventListeners, Containers, funções do PHP que usam callbacks (ex: array_map, array_filter, spl_autoload_register, etc etc.). Apesar delas terem um problema: não possuem nome, inclusive se você pegar a comunidade JavaScript de hoje, se usa muito menos closures do que antigamente. Callbacks tem seu uso, em poucos casos, eles levam à um código complexo, veja: jQuery.getScript("/js/foo.js", function() { jQuery.getScript("/js/bar.js", function() { jQuery(document).ready(function() { foo.run(bar.run()); }); }); }); E além disso você perde o tipo, perde a semântica e cai em um anti-pattern: obsessão por primitivos. Nada é especificado, a API não é sólida. Chega de teoria, vamos para prática. Primeiramente, o que é mais simples de se usar? veja: new AutenticadorPDO(new PDO, 'md5'); // vs.. new AutenticadorPDO(new PDO, new MD5); Óbvio que o primeiro, mas eu vou além, quando vamos usar SHA256, que precisamos usar a função hash com parâmetros: new AutenticadorPDO(new PDO, function ($senha) { return hash('sha256', $senha); }); // vs.. new AutenticadorPDO(new PDO, new SHA256); Qual é mais fácil de usar? Qual é mais fácil de testar (unit tests)? Qual é mais fácil de debugar? Qual é mais estável? Qual tem menos chances de erro? Qual é mais semântico? Qual segue o princípio "DRY"? Qual é mais fácil de ler? E também, em qual a implementação do Autenticador vai ser mais simples e limpa? Na segunda né.. Veja casos de uso reais de closures: $mediator->addEventListener('onSave', function ($user, $logger) { $logger->log('Usuário salvo: ' . $user->getName()); }); // ... $container->set('parser', function () { return new XMLConfigParser; }); $container->set('config', function () use ($container) { return new Config($container->get('parser')); }); // ... $router->get('/users', function () { echo 'Hello world!'; }); // ... spl_autoload_register(function ($className) { require sprintf('%s/src/%s.php', __DIR__, str_replace('\\', '/', $className)); }); // ... $schema->createTable('users', function ($table) { $table->pk('id', Table::AUTO_INCREMENT); $table->field('name', Table::VARCHAR, 255); }); // ... E vários outros casos de uso onde não há uma interface definida. Usar callbacks para strategy ou algo similar é tiro no pé. As closures são muito, mas muito úteis, mas não no caso onde há uma família de objetos de mesmo tipo, pois estaríamos destruindo o tipo e usando primitivos, genéricos, sem sentido. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 14, 2013 Do post 91; os autenticadores além de autenticar, geram a criptografia e existia repetição de código. Gera a criptografia apenas chamando a função de callback, ou seja, essa geração é delegada, não viola o SRP, bastaria eu chamar [inline]call_user_func_array[/inline] ali no próprio método autenticar e nem precisaria do método. Besteira é criar um objeto sem estado, sem atributos, com um único método que vai apenas encapsular uma função built-in pra ficar "mais OO". É como ter um parafuso pra apertar, ter uma chave de fenda manual em casa, mas sair de casa pra comprar uma que é automática, só porque vai reduzir o esforço. E vários outros casos de uso onde não há uma interface definida. Usar callbacks para strategy ou algo similar é tiro no pé. Em 99% dos casos, uma função de criptografia é uma função de um argumento string que gera uma string, pronto, está definida minha "interface". O PHP já implementa essas funções por padrão. No dia que eu precisar de algo mais sofisticado, que gere a criptografia com base em um par de chaves, uma pública e uma privada, aí sim eu crio uma família de algoritmos em classes e uso. Faria mais ou menos assim: class MyDigest implements Crypt { public function __construct($publicKey, $privateKey) { //... } public function apply($str) { // Faz a mágica... } public function __invoke($str) { return $this->apply($str); } } $controleLogin->autenticar(new AutenticadorPdo(new PDO(), new MyDigest('public', 'private'))); A flexibilidade do PHP ainda permite o seguinte: $crypt = new MyDigest('public', 'private') $controleLogin->autenticar(new AutenticadorPdo(new PDO(), array($crypt, 'apply'))); Assim eu não preciso nem do método mágico [inline]__invoke[/inline]. Ah, mas aí a interface não fica clara. Aí que entra uma coisa muito importante no desenvolvimento de software, mas que muitos se esquecem: documentação. Entenda que estou analisando o caso do ponto de vista de um programador PHP, visto que isso não pode ser aplicado em qualquer linguagem. Você não está errado, Enrico, mas na minha opinião, vale a máxima do engenheiro: O mais simples que funcione. Interessante a discussão pra mostrar que há mais de uma maneira de fazer, algumas são melhores, outras piores, existem as erradas, mas não existe só uma correta. Isso renderia bastante aqui, mas creio que estamos de novo nos desviando do tópico. Compartilhar este post Link para o post Compartilhar em outros sites
Enrico Pereira 299 Denunciar post Postado Abril 15, 2013 A minha implementação é simples... às vezes se cria um bicho papão sobre criar classes, eu tento abstrair ao máximo tudo e manter as coisas simples, segregadas e auto-descritivas e continua simples e limpo. Se uma API necessita de documentação, ela é ruim pois não é clara. Veja bem, eu disse *necessita*, uma API pode ter documentação, mas não deve ser um requisito. E sim, viola o SRP pois um autenticador não deve ser responsável pela criptografia. Criar tipos, separar responsabilidades, etc. não deixam o código complexo (quase nunca). Agora uma dica útil para quem for ler esse tópico: o que deixa um código complexo é o uso demasiado de herança e a violação da lei de Demeter. O foco, em qualquer discussão abstrata como esta, é comum de ser perdido. Compartilhar este post Link para o post Compartilhar em outros sites
felipe_zmm 0 Denunciar post Postado Abril 15, 2013 Enrico, Sua ideia não é ruim, mas eu particularmente me sinto desconfortável em criar classes que simplesmente chamam as funções do PHP, como o Henrique disse. Pra falar a verdade eu até acho que fica mais "bonito", porém me sinto dando uma volta desnecessária. Uma coisa que eu não compreendo é porque há tanta restrição em utilizar classes abstratas. Nesse caso eu acho que se aplicaria perfeitamente, já que os métodos em comum ficariam na classe abstrata e os autenticadores específicos implementariam o método autenticar cada um de sua maneira. Isso é o polimorfismo em ação, não ? Não vejo qual seria o problema de utilizar abstract em vez de interface. Talvez tenha alguma implicação que eu ainda desconheço. Ficaria assim: abstract class Autenticador { protected $cripto; public function __construct($cripto = null) { if ($cripto != null) { $this->cripto = $cripto; } } public function setCripto($cripto) { if (!is_callable($cripto)) { throw new Exception('Criptografia inválida!'); } $this->cripto = $cripto; } public function getCripto() { return $this->cripto; } public function criptografar($senha) { if ($this->cripto != null) { return call_user_func($this->cripto, $senha); } return sha1($senha); } abstract public function autenticar($login, $senha); } class AutenticadorPdo extends Autenticador { private $pdo; const SELECT_AUT = "SELECT COUNT(id) FROM usuarios WHERE login = :login AND senha = :senha AND status = '1'"; public function __construct(PDO $pdo, $cripto = null) { if ($cripto != null) { parent::__construct($cripto); } $this->pdo = $pdo; } public function autenticar($login, $senha) { $stmt = $this->pdo->prepare(self::SELECT_AUT); $stmt->bindParam(':login', $login); $stmt->bindParam(':senha', $this->criptografar(trim($senha))); $stmt->execute(); if ($stmt->fetchColumn() != 1) { throw new Exception('Usuário ou senha inválidos!'); } } } class AutenticadorTxt extends Autenticador { private $nomeArquivo; private $fp; public function __construct($arquivo, $cripto = null) { if ($cripto != null) { parent::__construct($cripto); } $this->nomeArquivo = $arquivo; } public function autenticar($login, $senha) { $senha = $this->criptografar($senha); $this->abrirArquivo(); while (!feof($this->fp)) { $arr = explode(', ', fgets($this->fp, 1024)); if (($login == $arr[0]) && ($senha == trim($arr[1]))) { $this->fecharArquivo(); return; } } $this->fecharArquivo(); throw new Exception('Usuário ou senha inválidos!'); } private function abrirArquivo() { $this->fp = fopen($this->nomeArquivo, 'r'); } private function fecharArquivo() { fclose($this->fp); } } Não mudaria nada na utilização das classes. A única coisa que não sei se está correta nessas classes é esse if pra chamar o construtor da super classe. Compartilhar este post Link para o post Compartilhar em outros sites
Henrique Barcelos 290 Denunciar post Postado Abril 15, 2013 E sim, viola o SRP pois um autenticador não deve ser responsável pela criptografia. Realmente, em teoria viola. Qual sua sugestão para que tenhamos um encriptador padrão? Acho que essa foi a intenção do Felipe. Se uma API necessita de documentação, ela é ruim pois não é clara. Veja bem, eu disse *necessita*, uma API pode ter documentação, mas não deve ser um requisito.Aqui você está redondamente enganado. Documentação é algo fundamental se você quer que sua API seja utilizada por terceiros. Por mais clara que a interface seja, ela jamais será intuitiva para alguém, se não o próprio autor do código. Ainda assim, se for algo que precisa de manutenção, com a documentação fica muito mais fácil pra você voltar nesse código daqui 2 anos e não se perder. Mas normalmente não se documenta por falta de tempo ou por preguiça mesmo. Compartilhar este post Link para o post Compartilhar em outros sites