Cecília e Cauê, obrigado pela avaliação. Abaixo segue nossa réplica em negrito, e aguardamos a sua tréplica!
--
O layout cou excelente, apenas sentimos falta de botões de voltar e cancelar
ações.
Mesma reclamação da outra equipe. Vamos arrumar!
FrontController: não entendemos a motivação do método executaAcao - a idéia
de separar as ações da parte de visualização está bem de acordo com o MVC,
mas se o nome da URL é o parâmetro passado para o request dispatcher, a
dependência se mantém. A con guração de página de erro poderia ser feita
diretamente no web.xml com a tag <error-page>.
Não entendi a dúvida sobre o método executaAcao, no servlet de FC. A idéia dele, assim como a sua implementação, é setar "request" e "response" da ação, executá-la, e aí disparar para a view correta. O que é disparado para o RequestDispatcher é o nome da view devolvida pelo Action.
BaseAdminAppController: essa classe deveria ser um ltro que rege apenas
sobre os usuários que tentam acessar lógicas de administradores. Simpli caria
a implementação sem adicionar mais uma camada de classes abstratas.
Sim, realmente poderíamos ter optado pela implementação de um filtro, ficaria até mais claro. Com certeza seria a melhor opção. Optamos pela implementação mais simples apenas para facilitar o trabalho.
FabricaDeRepositorios: o nome "Fábrica"foi utilizado de forma errônea.
Tanto o padrão de projeto Fábrica como citado no GoF e sua versão que utiliza
interfaces tem como idéia fornecer uma classe genérica que devolve um objeto
com características especí cas, de acordo com uma con guração.
Nossa fábrica não funciona exatamente como a implementação do GoF sugere pois nosso problema de criação é muito simples. Temos a mesma motivação (criar um objeto), mas não vejo o porquê de complicar algo que pode ser simples! O código é bastante legível, não creio que alguém teria problemas em entender.
Obviamente que poderíamos carregar de um arquivo de configuração qual implementação concreta ele deveria criar e tudo mais, mas deixamos isso mais pra frente, pra quando vermos o Spring!
O nome do padrão usado na FabricaDeRepositorios existe e conhecido como Static Factory
(http://today.java.net/pub/a/today/2005/03/09/factory.html#static_factory).
Ele é inclusive um dos primeiros itens citados pelo Joshua Bloch no
Effective Java (http://www.javaworld.com/javaworld/jw-06-2001/j1-01-sintes2.html).
Isso explica porque demos o nome de fábrica para essa classe e porque
usamos os métodos estáticos.
BaseRepositorioJdbc: não entendemos a necessidade das veri cações de co-
nexão nula nessa classe. Se de fato for a idéia, como parece, de sempre que
terminar de usar o código fechar a conexão, por que não abrí-la e fechá-la
sempre em cada método do repositório? A ausência dos "recebe null"e "se
é nulo..."deixaria o código mais elegante.
Hmmm, nada como outro olho pra revisar o código. Se pudesse argumentar, diria que isso foi algum refactoring mal feito que executamos! Talvez tivéssemos pensado em não fazer a própria base cuidar da conexão com o banco de dados, e depois optamos por implementá-la, e aí o código realmente ficou confuso. Vale o refactoring. E ufa, ainda bem que existem o tal do hibernate, não? :P
-- Outro comentário --
Realmente, ao deixarmos a cargo dos métodos de repositório fechar
mas não abrir as conexões deixamos de fazer um bom uso de composed
methods (http://c2.com/ppr/wiki/WikiPagesAboutRefactoring/ComposedMethod.html),
e de certa forma ferimos o SLAP
(http://books.google.com.br/books?id=oVadw8Jf0cYC&pg=PA160). Mas
BaseRepositorioJdbc.finaliza(), invocado no finally, precisa checar se
a conexão é diferente de null porque uma exceção pode ocorrer
justamente na abertura da conexão. Caso isso não seja feito,
poderíamos ter uma NullPointerException. Uma melhoria que realmente
pode ser feita é a checagem da conexão em
BaseRepositorioJdbc.conecta(), ao invés de checar nos métodos que
fazem uso de BaseRepositorioJdbc.conecta(). No fim temos consciência
de que esse é um esquema bastante primitivo de gerenciamento de
conexões com DB.
Ranking: não há necessidade da separação do pacote Ranking do restante dos
repositórios, já que ele também se parece muito com um repositório - até lança
RepositorioException. Também vale lembrar que repetir um mesmo método em
dois lugares é um má idéia, já que di culta a manutenção posterior, se necessária
(método "rankeia").
Hmmm, lançar RepositorioException é realmente falha nossa, repare que as duas classes concretas que implementam "Ranking" não lançam "RepositorioException". (Mais uma vez problema no refactoring, pois na primeira implementação, Ranking realmente batia em um repositório para pegar os dados, e mesmo assim, deveriam lançar algo do tipo "RankingException").
Não acho que ele seja um repositório, acho que ele se aproxima mais de um serviço, ele nem usa o repositório! Ele recebe uma lista de posts, e a partir de um algoritmo (como selecionar as maiores notas), devolve uma outra lista.
Sobre o mesmo método e manutenção posterior, não entendi!? Cada ranking (Melhor e Pior) implementa sua própria regra de negócio. A idéia é que pra aplicação "é tudo ranking" !
Actions em geral: as actions de uma aplicação fazem parte do modelo, e não
do controlador. Chamá-las de "AppController"dá uma falsa impressão da parte
do código em que estamos trabalhando.
Opa, essa discussão aqui vai longe! Acho que Actions não fazem parte nem do modelo nem do controlador. Acredito que eles constituem uma outra camada, responsável por controlar a interface e interagir com os objetos de domínio. Acho que nossos AppControllers estão perto do que a Sun diz que é o padrão (http://www.corej2eepatterns.com/Patterns2ndEd/ApplicationController.htm), correto?
Os nomes da maioria dos métodos e classes estão bastante signi cativos, elimi-
nando a necessidade de comentários. Parabéns.
Obrigado!
As 23 classes com su xo "AppController"são poluição desnecessária para
quem vê os nomes das classes. Seria bastante mais econômico e visível tirar esse
su xo.
Podemos colocar o sufixo "AC". O que acham?
Algumas variáveis foram chamadas "b", "rs"e ans. Com as ferramentas de assistência de código, escrever nomes signicativos, ainda que longos, não é problema. Dessa forma, a legibilidade do código aumenta consideravelmente.
Sobre os nomes de variáveis, realmente economizar no nome de
parâmetros não é uma boa ideia. Algumas variáveis tem alguns nomes como "p" e tudo mais. Isso será corrigido.
Mas no caso específico de rs e stmt, isso é um tipo de expressão comum em programação JDBC. Um exemplo é o próprio tutorial de JDBC da Sun
(http://java.sun.com/docs/books/tutorial/jdbc/overview/index.html) e o
Java Almanac(http://www.exampledepot.com/egs/java.sql/GetRsData.html).
Seguindo por essa linha, talvez devêssemos ter usado 'con' ao invés de
'conexao'.
BaseRepositorioJdbc: para comportarem-se como constantes, basta o uso
da palavra-chave final, o static não é necessário
É uma convenção de Java declarar constantes cujo valor é conhecido
em tempo de compilação como static final
(http://mindprod.com/jgloss/constant.html). Essa prática tem por
objetivo alocar memória para uma única instância para toda a VM, ao
invés de uma por instância da classe que declara a constante. Alguns
exemplos do próprio JDK são Boolean.TRUE / FALSE
(http://java.sun.com/javase/6/docs/api/index.html?java/lang/Boolean.html)
e GroupLayout.DEFAULT_SIZE
(http://java.sun.com/javase/6/docs/api/index.html?javax/swing/GroupLayout.html).
FabricaDeRepositorios: os métodos poderiam, sem perda de legibilidade,
não serem estáticos.
Como explicado acima, optamos pelo uso do Static Factory.
Mesmo assim, vocês acham que o código "RepositorioX repositorio = new Fabrica().criaRepositorioX();" fica melhor do que ela sendo estática?
O problema de inserir muitos static no seu código é dar uma cara procedural
para um código que deveria ser orientado a objetos. Tente evitar essa prática
no futuro! =)
Hmm, argumentamos sobre isso nos tópicos acima!
BaseRepositorioJdbc: usar sempre o PreparedStatement é uma boa idéia, já
que ele é a classe que surgiu para substituir o Statement e previnir por padrão
a velha e conhecida SQL Injection.
Usamos Statement somente em consultas onde não há entrada de dados feita pelo usuário. Repare que sempre que há entrada do usuário, foi utilizado o PreparedStatement.
Nos modelos, o uso da classe Date é não recomendável em Java, já que esta
foi uma primeira implementação e vem sido deprecada com o tempo. Pre ra
usar Calendar - que é internacionalizável e tem métodos que facilitam lidar com
as datas.
Faz realmente muito tempo que Date está depreciada. Vamos corrigir.
Uma página de erro poderia ter sido con gurada. Levamos uma stacktrace de
erro 500 ao tentar acessar a parte de login da parte de administração porque no
Readme faltava a parte "/admin".
Vamos arrumar no README.
As funcionalidades atendem e até excedem às expectativas. Excelente produto
nal entregue.
Obrigado! Não é um WordPress da vida, mas... ;)
--
Abraços!