Refatorando o JForum - ajuda com a arquitetura

34 respostas
Rafael_Steil

Atencao: A mensagem que se segue ficou um tanto grande, e nao cobre todos os pontos que gostaria. Portanto, ao inves de fazer um livro com as minhas duvidas, vou faze-las em partes, de acordo com o andamento do topico.

Atualmente, a logica de negocios do JForum esta basicamente nas actions - como “logica de negocios” aqui, por favor entendam coisas como verificacao de seguranca, validacao de parametros, definicao do contexto do freemarker e acesso aos DAOs. Ha bastante codigo em classes utilitarias tambem, as quais foram movidas para tais pois eram usadas em mais de um lugar. Nao vou entrar em detalhes de implementacao por nao haver mta necessidade.

Para sair dessa meleca, comecei a refatorar o JForum. A cada hora que passa vejo cada vez mais que estou a ponto de reescrever uma enorme parte do codigo fonte. Embora o codigo atual esteja limpo e, principalmente, facil de entender e manter, falta pouco para um limite da arquitetura, onde entao ficara muito dificil adicionar coisas novas sem prejudicar a estabilidade do sistema (aka, falta de testes da nisso).
Conversando com o Villela e dando uma lida no PoEAA, GOF e Core J2EE Patterns (fora material da net), acabei ficando com muitas duvidas em relacao a como fazer o refactoring. De uma maneira ampla, alem dos ja citados elementos “extensiblidade” e “testabilidade”, faz-se necessario ter uma API de servicos, afim que seja possivel interagir com o JForum usando meios outros que somente um browser.

Tendo isso em mente, o que tenho (praticamente) definido eh assim:

:arrow: As actions somente irao interagir com os servicos, e fazer coisas especificas ao ambiente - no caso de web / browser normal, eh configurar o contexto do sistema de templates / criar objetos a serem passados aos servicos.

:arrow: Sobre “servico” eu entendo como um conjunto de metodos que servem como ponto de entrada / delegadores para a camada de negocios.

Ai comecam a pintar as maiores duvidas, sendo um tanto dificil achar um ponto de partida. Vou explicar o que comecei a fazer, tentar dar alguma justificativa, e esperar que alguem aqui tenha uma boa ideia. A primeira ideia que eu tive foi, obviamente, tirar todo codigo “de negocios” das actions, e mover para classes de servico, como PostService, TopicService e etc… tais classes verificam se o usuario tem direito de execucao na tarefa, se o topico solicitado existe, e entao chamam os metodos dos DAOs para fazer a persistencia. Simplificando o codigo, fica algo como

// PostService.java#delete(int postId)
if (!SecurityRepository.canAccess(SecurityConstants.PERM_MODERATION_POST_REMOVE)) {
	throw new ServiceException(ServiceErrors.POST_CANNOT_DELETE);
}

// ...

PostDAO pdao = DataAccessDriver.getInstance().newPostDAO();
Post p = pdao.selectById(postId);

// ...

pdao.delete(p);

// ...

DataAccessDriver.getInstance().newUserDAO().decrementPosts(p.getUserId());
tdao.decrementTotalReplies(this.underlyingPost.getTopicId());

int minPostId = tdao.getMinPostId(p.getTopicId());
if (minPostId > -1) {
  tdao.setFirstPostId(p.getTopicId(), minPostId);
}

// ...

fdao.setLastPost(p.getForumId(), maxPostId);

Ou seja, o servico se encarrega de cuidar das dependencias e tudo mais. Porem, esse codigo nao esta me agradando muito, pois ainda parece um codigo bastante tanto acoplhado e especialmente fragil. O cv sugeriu usar listeners e mover boa parte da logica para os pojos, formando assim um domain model (ou algo parecido com isso). Indo por esse lado, o codigo anterior resultaria nisso:

Define o listener

interface PostListener {
	void postDeleted(Post p);
}

Classes que precisam ser notificadas sobre alteracoes em posts implementam-a

interface UserDAO extends PostListener { ... }
class ConcreteUserDAO implements UserDAO {
	void postDeleted(Post p) {
		this.decrementPosts(p.getUserId();
	}

	void decrementPosts(int userId) { .... }
	// ...
}

O mesmo para o topico

interface TopicDAO extends PostListener { ... }
class ConcreteTopicDAO implements TopicDAO {
	void postDeleted(Post p) {
		int totalPosts = this.getTotalPosts(p.getTopicId());
		if (totalPosts < 1) {
			this.delete(p.getTopicId());
			return;
		}
		
		int maxPostId = this.getMaxPostId(p.getTopicId());
		this.setMaxPostId(maxPostId);

		int minPostId = this.getMinPostId(p.getTopicId);
		this.setMinPostId(minPostId);
	}
}

A classe de post se encarrega de notificar todo mundo

class Post {
	static List listeners = // ...

	void delete() {
		PostDAO dao = // ...
		dao.delete(this);
		this.notifyPostDeleted();
	}

	void notifyPostDeleted() {
		for (PostListeler pl : listeners) {
			pl.postDeleted(this);
		}
	}

Os observers sao registrados no startup do sistema

Post.addListener(new ConcreteUserDAO());
Post.addListener(new ConcreteTopicDAO());

Removendo uma mensagem.

Post p = // ...
p.delete();

A BEM grosso modo eh isso. Nao tenho uma implementacao concreta, mas tenho varias duvidas. Dicas gerais sobre formas de estruturar o codigo, responsabilidades e tudo mais sao muito bem vindas.

Rafael

34 Respostas

cv1

Bom, acho que eu nem tenho muito a adicionar aqui… mas pq vc tem uma interface pros DAOs? Se, a principio, vc so tiver uma implementacao, use a impl direto (dado que o seu model nao vai usar o DAO diretamente, ele so vai notificar um Listener - que por acaso eh um DAO). Assim voce evita classes com nomes medonhos do tipo FooImpl ou ConcreteFoo :slight_smile:

duardor

E depois se precisar trocar a impl faz um refactoring no código todo…??? Eu tenho opinião contrária… Eu acho q devia continuar com as interfaces…

cv1

O que tem de tao dificil em UM extract interface, Eduardo?

duardor

Soh existe UM DAO???
E pq não fazer do “jeito correto” logo??? Qual e a vantagem de fazer o extract interface depois?

cv1

Nao eh taaao importante assim, mas o ponto eh que, se vc vai ter uma interface com uma unica implementacao, entao pra que a interface? “Ah, eu posso ter uma outra implementacao qualquer dia desses”? Se esse dia nao chegar, vc programou a toa :wink:

Rafael_Nunes

E se esse dia chegar?

Thiago_Senna

simples… vai lá e muda!!!

cv1

Se esse dia chegar vc perde os 5 minutos que vc precisa pra dar os extract interface nos lugares necessarios, e implementa os novos DAOs, ueh! Voce vai ter que revisar as interfaces de qualquer jeito, pq fatalmente a nova implementacao vai ter alguma diferencazinha que vc vai ter que acomodar pra cobrir todos os casos sem fazer muito esforco. :wink:

F

XP, XP, XP é isso que o CV ta tentando dizer.
Faca o que precisa hoje, se um dia precisar mudar algo vai la e muda. Ou seja se precisar de um DAO novo extract interface, enquanto esse dia nao chega deixa o sistema com uma interface a menos.

]['s

Rafael_Steil

Pelamordedeus… voces tao brigando por causa dos daos? :mrgreen:

Eu tenho interfaces para eles pq nao uso Hibernate e preciso suportar diversas fontes de dados (suporta ja mysql, hsqldb, postgresql, oracle e sqlserver).

Nao, uma migracao para hibernate nao esta nos planos no momento :wink:

Rafael

btw: eh MySqlForumDAO, OracleForumDAO etc… mas isso quem me traz eh o abstract factory.

duardor

Ah nem…
Acho que é muito estilo de programação isso aqui… Cada um acha melhor do seu jeito… Eu prefiro abstrair e esconder atrás de interfaces desde o primeiro momento…
Sejam felizes aqueles que nâo gostam :stuck_out_tongue: :stuck_out_tongue: :stuck_out_tongue:
Mas sei lá, acho que programar classes sem interfaces eh menos OO do que programar com interfaces…
Talvez seja purismo demais, mas eu gosto do código com mais interfaces (sem exageros, mas eu acho que no caso de daos se justifica sim)…

T+

duardor

Rafael Steil:
Pelamordedeus… voces tao brigando por causa dos daos? :mrgreen:

Eu tenho interfaces para eles pq nao uso Hibernate e preciso suportar diversas fontes de dados (suporta ja mysql, hsqldb, postgresql, oracle e sqlserver).

Nao, uma migracao para hibernate nao esta nos planos no momento :wink:

Rafael


Brigando nada! Eh soh uma discussãozinha bem construtiva…
:slight_smile:

T+

cv1

Bom, nesse caso, voce ja TEM mais de uma implementacao. Caso encerrado :mrgreen:

cv1

Em Smalltalk, uma linguagem que ninguem aqui discorda que eh 100% OO, nao existem interfaces. E ai, como fica? :wink:

O ponto eh que interfaces sao soh contratinhos que vc coloca na frente das suas classes. Eh o jeito que o Java arrumou pra fazer heranca multipla, mas nao eh algo OBRIGATORIO, ou que deveria te deixar desconfortavel. Se nao precisar ter, nao precisa ter, e pronto. Menos codigo pra escrever, documentar, depurar e manter :smiley:

duardor

Em Smalltalk, uma linguagem que ninguem aqui discorda que eh 100% OO, nao existem interfaces. E ai, como fica? :wink:

O ponto eh que interfaces sao soh contratinhos que vc coloca na frente das suas classes. Eh o jeito que o Java arrumou pra fazer heranca multipla, mas nao eh algo OBRIGATORIO, ou que deveria te deixar desconfortavel. Se nao precisar ter, nao precisa ter, e pronto. Menos codigo pra escrever, documentar, depurar e manter :D
Como eu disse antes, bem do gosto do fregues…
Como fica akela baboseira toda da OO sobre programacao orientada por contratos nessa linguagem???

pcalcado

Contratos são muito melhor expressos em pós e pré condições do que simplesmente com assinaturas de métodos. Smalltalk eu não sei, mas Eiffel (e até mesmo aquela linguagem de constraints maldita da UML) tem como forma de estabelecer contratos:

  • Inveriante da classe
  • Pós Condição por método
  • Pré-condição por método

Quando você chama um método ele checa a pré-condição, executa o método, checa a pós e após isso checa a invariante. Isso sim é um contrato obedecido :slight_smile:

Bom, o tópico descambou de assunto, as ever, ma Steil… pra mim tá legal. Infelizmente não liberam a droga da porta do CVS daqui pra eu poder fazer um checkout :frowning:

caiofilipini

A OCL (Object Constraint Language)? :twisted:

Rafael_Steil

Ah, finalmente voltamos ao assunto :D.

O lance de usar listeners eh uma boa entao?! Legal, show. Agora entra a segunda parte da duvida:

Algum problema em ter alguns metodos na Post, como delete() / update() (que geralmetne precisam de notificacoes / validacoes mais pesadas) e, nos outros casos (comos selects de utilidade geral), acessar o DAO diretamente quando precisar? (“diretamente” aqui seria o servico que chamaria).

Eh ok assim? Por um lado ficaria um pedaco do trabalho em um canto e o resto em outro canto, e nao teria como impedir que alguem chamasse o dao direto (bom, claro que eu nao faria isso, mas vai saber quem vai colocar a mao no codigo).

Ai, complementando essa pergunta, estava pensando em deixar as validacoes no servico… tipo, verificar se o conteudo existe, se o usuario tem direito de acesso etc etc… Como todo mundo sempre ira passar pelo service - inclusive a api externa -, me parece um bom lugar… Eh ok?

Valeu
Rafael

fsamir

Dae,

acho q esta discussão sobre usar interfaces ou não está poluindo o tópico.

Rafael, gostei da idéia do cv de usar Listeners. Sugiro usar AOP para isto.

Acho q só adicionar uma camada de Business Objects já melhora bastante. Normalmente eu diria para usar Delegates nos casos de lógicas mais complexos, mas a opção dos Listeners me parece melhor neste contexto.

Fora isto tenho apenas duas considerações a fazer:

  1. Tu já está cansado de ler isto mas, eu vou repetir denovo só para não perder a fama de chato: A camada de persistência é a mais complicada na minha opinião. Usar um framework de persistência tornaria muito mais fácil adicionar e MANTER o suporte ao um novo BD. Concordo q o refactoring está em primeiro plano mas, não deixaria de colcoar em segundo plano o uso de um framework de persistência.

  2. Desenvolvimento mais focado em Test Case. O refactoring vai tornar mais simples o uso de Test Cases, eu sei. Apesar disto poderíamos investir mais nisto, principalmente por causa dos DAOS. Sem Test Cases o trabalho manual de testar cada implementação se torna inviável.
    Eu gosto muito de testar deste a camada controller. No Spring é bem fáci lde fazer isto, a getne podia pegar um pouco da mesma idéia e implementar no JForum.
    Olha um test de action usando Spring:

...
 NewsMultiAction action = (NewsMultiAction) ac.getBean("newsViewController");
        ModelAndView mav = action.handleRequest((HttpServletRequest) null,(HttpServletResponse) null);
        Map m = mav.getModel();
        assertNotNull(m.get("newsList"));
        assertEquals(mav.getViewName(), "/News/list");

Assim eu consigo testar até se a página resultante é realmente a página que eu quero. Não chega ao ponto de ver se o HTML está quebrado, nem é este o objetivo e seria fácil de adicionar caso fosse necessário.

cv1

Nenhum problema - vc pode separar os DAOs, inclusive: um pra fazer queries e outro pra fazer modificacoes, e deixar os DAOs que modificam coisas escondidinhos dentro das entidades do modelo :wink:

Nao era mais facil fazer a validacao ser um punhado de Listeners que roda antes dos DAOs? Assim, o seu domain model continua coeso e representando as entidades direitinho, e o Service Layer cuida de prover as queries :slight_smile:

cv1

Obrigado, obrigado :mrgreen:

Qual o uso de AOP que tornaria a coisa melhor aqui? Colocar o mecanismo de observer/observable encapsulado em um aspecto separado?

Apoiadissimo :smiley:

Nao diria “mais focado em TestCase”, pq no fim das contas o JForum eh um forum, nao um monte de barrinhas verdes e felizes. Mas… o monte de barrinhas verdes e felizes ajudaria o JForum imensamente a ser um forum cada vez melhor :wink:

louds

A idéia de usar listeners é legal, mas manter isso estáticamente é uma péssima idéia. Não sei como está hoje, mas os DAOs são 100% stateless? Por que se não forem você vai ter problemas.

Nesse primeiro momento não precisa, mas seria bom mover a configuração dos listeners para fora do código java, facilitando a configuração.

Outra coisa, validação usando listeners não me parece uma boa idéia, mesmo usando o conceito legal de VetoListeners do modelo de eventos dos JavaBens. Não seria melhor prover esse tipo de funcionalidade, como validação, atraves de decoradores?

Ou seja, antes do teu serviço executar, uma série de decoradores vão realizar tarefas como validar permissões, existencia e tudo mais.

E qual a vantagem de usar decorator em vez de listeners? Primeiro que fica menos intrusivo para o serviço o fato dos decorators existirem, segundo que somente a Factory dos serviços precisa se preocupar em decorá-los. A desvantagem é que precisa tormar cuidado quando passar this como parâmetro.

louds

Não sei se esse código existe ou não, mas o correto não seria usar updates e deletes verificando o número de registros alterados. Tem a interação com caching, mas fica parecido.

Atualizar o maxPostId

update topics set max_post_id = ? where topic_id = ? and max_post_id < ?
ou se o banco tiver uma função de seleção de maior valor (mysql):
update topics set max_post_id = GREATEST(max_post_id, ?) where topic_id = ?

Para o caso de apagar topicos sem posts:

delete from topics where topic_id = ? and not exists (select * from posts where posts.topic_id = topics.topic_id)
cv1

+1 pra ideia dos decorators - fica mais limpinho, mesmo… ai voce pode espetar outros tipos de tratamento dos dados nos listeners (ou fazer coisas emocionantes tipo logging), e tambem te ajudam a tirar a logica dos DAOs.

Ao inves de fazer um metodo no DAO que apaga um post, decrementa o numero de posts do usuario, decrementa o numero de posts da thread, decrementa o numero de posts do topico e frita uns bifes, vc faz varios listenerzinhos que escutam a mensagem postDeleted :wink:

louds

cv:
+1 pra ideia dos decorators - fica mais limpinho, mesmo… ai voce pode espetar outros tipos de tratamento dos dados nos listeners (ou fazer coisas emocionantes tipo logging), e tambem te ajudam a tirar a logica dos DAOs.

Ao inves de fazer um metodo no DAO que apaga um post, decrementa o numero de posts do usuario, decrementa o numero de posts da thread, decrementa o numero de posts do topico e frita uns bifes, vc faz varios listenerzinhos que escutam a mensagem postDeleted ;)

Só toma cuidado para não trocar spaghetti por raviolli.

renatosilva

Caras quando vejo essas coisas, eu fico me perguntando: “putz aonde e com quê eu fui me meter…” :mrgreen:

Rafael_Steil

Bom, o update pro max post id sempre vai ter que existir, independente da maneira como eh feito. Em bancos que suportam subqueries, ate da para automatizar com um

update topics set max_post_id = (select max(post_id) from .... ) where topic_id = ?

mas ainda assim preciso ter um metodo que pega o max post manualmente, para os casos de bancos sem subqueries (aka mysql < 4.1).

Em relacao a remover o registro da topics caso nao haja posts, a diferenca do teu exemplo eh que voce manda dar o delete de cara, sem verificar antes de eh necessario ou nao… nao chega a ser uma big coisa.

Rafael

Rafael_Steil

cv:
+
Ao inves de fazer um metodo no DAO que apaga um post, decrementa o numero de posts do usuario, decrementa o numero de posts da thread, decrementa o numero de posts do topico e frita uns bifes, vc faz varios listenerzinhos que escutam a mensagem postDeleted ;)

Bom, mas no fundo ainda existiria os respectivos metodos no dao, nao?! (mesmo que eles fossem chamados por algum observer)

Rafael

Rafael_Steil

Bom, de fato vai ter que ficar para o futuro. Eh inviavel levar sozinho dois refactorings pesados assim, que demandam um grande esforco para codificar e testar.

HHm… teste da interface web? (ou proximo disso?)… o Selenium eh mto bom, e com certeza vou usa-lo.

Rafael

louds

Rafael Steil:

Bom, mas no fundo ainda existiria os respectivos metodos no dao, nao?! (mesmo que eles fossem chamados por algum observer)
Rafael

Eu vejo a camada de DAO fazendo somente o plumbing entre banco e aplicação. Os métodos devem ser, em geral, atômicos e não produzir qualquer efeito colateral além do óbvio.

Então sim, os daos continuariam tendo os métodos para falar com a base de dados e fazem nada além disso.

Quanto ao comentario sobre o delete. Eu ignorei detalhes sobre caching para simplificar a coisa, supondo que você tem somente o topic_id, não tem como evitar ir ao banco, certo? Essa era minha sugestão, se precisar fazer um select count() melhor mandar o delete ou update direto. Com mysql < 4.1 acho que rola fazer uma gambiarra com multi-table delete/update para burlar a falta de subqueries.

Rafael_Steil

Certo, concordo.

Em relacao ao caching, o que eu comecei a fazer tambem eh criar uma implementacao cached do DAO, que fica na frente da implementacao concreta. Atualmente, o codigo esta assim:

forumDao.delete(forumId);

ForumRepository.removeForum(forum);

Com o refactoring, somente vai existir a chamada ao delete() do ForumDAO. A estrutura eh assim:

ps: isso, obivamente, fica definido nos arquivos de configuracao. Ai, o CacheForumDAO fica assim:

void delete(forum) {
    concreteDao.delete(forum);
    ForumRepository.remove(forum);
}

o cliente nunca ira saber da existencia do cache, ja que vai solicitar uma instancia do dao ao factory e chamar um “unico” metodo.

Rafael

louds

Isso é um decorator, exatamente. Legal.

R

Estava lendo este tópico antigo e gostaria saber se atualmente o código do Jforum que está no Github pode ser considerado um bom exemplo de arquitetura ou ainda há muito código da versão antiga que precisa de melhorias e refatoração ?

Estou procurando alguns projetos maiores de código aberto usando padrão MVC para verificar como foram feitas as implementações do controller, serviços , autorização, etc. Exemplos mais didáticos as vezes não mostra muito claramente como resolver alguns problemas mais práticos, por exemplo, uma view que tem um grande número de dependências, no JForum, por exemplo, há construtores com uns 10 parâmetros.

Rafael_Steil

robsp,

a maior parte do código do JForum 3 é novo, e que foi reutilizado foi aprimorado na medida do possível até o momento. Tem algumas coisas lá (como o esquema de serviços) que estou reavaliando a implementação, pois o que tinha em mente no início era ter toda uma API desconectada do ambiente Web, mas hoje em dia vi que era algo meio idiota querer chegar em tal ponto. Isso não significa que está ruim, é mais uma questão conceitual.

A respeito dos construtores dos Controllers, ao meu ver não tem muito como fazer diferente… talvez, em alguns casos bem especificos, daria para diminuir esse número se algumas das dependências fossem diluídas por outros componentes, mas isso precisa ter uma finalidade bem específica. Querer diminuir só porque parece ter “bastante” é um motivo muito fraco, que pode acabar causando confusão em outras pontas se você não prestar atenção na maneira de refatorar.

Rafael

Criado 11 de abril de 2005
Ultima resposta 7 de mar. de 2012
Respostas 34
Participantes 12