Opinião dos mais experientes: refatoração amigável x desacoplamento?

[bronx]

Senhores, boa noite!

Essa semana, durante a divisão das atividades, meu sócio ficou encarregado de desenvolver um cadastro qualquer no sistema que estamos criando.

Ele desenvolveu a solução, mas fez de tal maneira que a JSP usada na tela de cadastro é a mesma utilizada na tela de alteração dos dados cadastrais. Não só a JSP. Os métodos utilizados na classe do servidor (estamos utilizando o Vraptor 3) são os mesmos para ambas as operações.
Até aí, sem problemas. Não digo que nos poupou trabalho, pois a página de alteração seria um simples CTRL-C, CTRL+V do que ele havia feito.

Porém, sempre fui do time contrário a essas paradas. Deixar tudo centralizado no mesmo local me soa um tanto quanto procedural. E digo com razão da palavra, pois já trabalhei mais de 2 anos com mainframe (COBOL, CICS… :oops: hehehe).
Falei para ele fazer em arquivos (JSPs) separados e criar os respectivos métodos no controller. Porém, o argumento dado para não fazer isso foi de que seria um problema na hora de alterar alguma coisa em uma página (refatorar), pois teríamos que replicar as alterações em ambos arquivos.

Justo. Até certo ponto.

Algum tempo depois (alguns minutos, para ser mais preciso…rs), tive que fazer uma alteração na bendita página. O problema foi justamente que a vantagem da refatoração amigável acabou indo por água abaixo quando precisei alterar o método que controlava as operações. O bendito método estava sendo acessado, em ambas operações (inclusão e alteração), pela mesma URI: “/cadastrar”. Alterei, de forma que, quando fosse uma alteração, fosse passada na URI o id do objeto em questão: “/cadastrar/idDoObjeto”.
Aí já causa um desconforto, pois a semântica fica um pouco confusa. O usuário pode pensar: “Cadastrar um objeto que já está cadastrado?” (???).
Porém, isso azedava o pé do frango em relação a inclusão, que era acessada pela URI “/cadastrar”. Ou seja, a questão da refatoração amigável acabou se mostrando não tão amigável assim nessa situação, pois o alto grau de acoplamento entre as actions acabava por destruir esse benefício.

Mas, mesmo assim, meu sócio não quer alterar a estrutura da solução, criando arquivos separados e métodos distintos para cada situação. Me sugeriu até mesmo criar um campo hidden passando qual a ação estava sendo tomada naquele momento. Nem vou comentar o que penso a respeito disso.

Além disso, sugeriu que eu faça a mesma coisa com todas as minhas páginas de cadastros, alegando o supracitado “benefício” da refatoração.

Enfim, me digam aí o que vocês pensam a respeito disso. É nítido que prefiro deixar tudo separado, deixando o mais desacoplado possível. Mas posso estar equivocado e não ter enxergado algum problema mais profundo quanto a essa abordagem. O que preciso é de argumentos que me convençam que estou errado, ou que possam convencê-lo de que, nesses casos, o desacoplamento é mais adequado.

Valeu galera.
Aguardo comentários.

Abs.

[guedes]

Olá bronx,

Mas, mesmo assim, meu sócio não quer alterar a estrutura da solução, criando arquivos separados e métodos distintos para cada situação. Me sugeriu até mesmo criar um campo hidden passando qual a ação estava sendo tomada naquele momento. Nem vou comentar o que penso a respeito disso.

Deixa que eu comento por você.
É POG e ponto.
xD

Brincadeiras a parte, concordo com você, sou a favor de deixar separado.
Sempre aprendi (e também sempre considerei o ideal) que cada Classe/Método/Objeto tem que ter suas responsabilidades bem definidas.

Se você tem um método cadastrar seu nome já é bem objetivo e claro em relação a sua funcção, não faria o menor sentido chamar

obj.cadastrar(cliente)

para editar os dados do cliente.

Eu não consideraria, nesse caso, o argumento do seu sócio válido.
Imagie a situação:
Você precisa fazer uma alteração na rotina na hora de alterar o cliente, não cadastrar, o cadastro continuaria exatamente o mesmo, então você iria pensar em qual método você precisa alterar, como você desenvolve a aplicação você sabe que esse método é o ¿cadastrar¿.
A meu ver isso não é nem um pouco amigável, amigável seria ir até o método editar/alterar/edit.

Outra situação que isso atrapalharia seria a inclusão de mais um profissional na equipe.
Se ninguém o alertar disso ele pode ficar um bom tempo procurando o método editar/alterar.

Em relação aos usuários acho que não seria um problema tão grande, acredito que a grande maioria nem presta atenção nas URL’s de sites e aplicações.

Novamente batendo na mesma tecla, o ideal seria, pelo menos pra mim, sempre seguir a boa prática de dar bons nomes nas Classes, Métodos, Atributos, Objetos, …
E nenhum nome é melhor do que aquele que diz o que ele realmente faz.

Mas ai cairia na situação de alterar o nome do método para cadastrarEAlterarCliente(…).
Ninguém mais vai poder falar que o método não faz o que o nome diz, mas do mesmo jeito vai quebrar a questão das responsabilides.
Daqui a pouco o cara ta programando a aplicação inteira no doGet(…) da Servlet.

Vou ficando por aqui que já me estendi demais.

[bronx]

Exatamente guedes.

Sei que existem situações que de fato há um ganho em relação a refatoração. Mas, de longe, não são esses que citei.

Acredito que seja vício em acreditar que é sempre vantagem economizar código. Mesma coisa com os famigerados DAOs genéricos.

Mas enfim, alguém mais poderia dar uma opinião aqui?

[bronx]

Alguém mais??

[bronx]

Ninguém?

[Abdon]

Ola, acho que vc teve poucas respostas, por que pelo menos para mim ficou dificil visualizar o que seu socio fez.

Ele fez tudo em jsp? Acesso ao banco de dados e etc?

Ele usou o mesmo formulario para os dois casa, sendo que o resto da logica esta nas suas classes de negocio que são disparadas pelo vRaptor?

Escrever alguem mais ou ninguem em um forum é dar um up na sua mensagem, o que segundo a netiqueta é considerada falta de educação.

[bronx]

ovelha,

Ele está utilizando a mesma jsp e mesmo método tanto para salvar quanto para alterar os dados.
Criei o tópico esperando obter opiniões que o façam mudar de idéia.

Sacou?

Valeu!!

[]'s

[sergiotaborda]

[quote=bronx]Senhores, boa noite!

Essa semana, durante a divisão das atividades, meu sócio ficou encarregado de desenvolver um cadastro qualquer no sistema que estamos criando.

Ele desenvolveu a solução, mas fez de tal maneira que a JSP usada na tela de cadastro é a mesma utilizada na tela de alteração dos dados cadastrais. Não só a JSP. Os métodos utilizados na classe do servidor (estamos utilizando o Vraptor 3) são os mesmos para ambas as operações.
Até aí, sem problemas. Não digo que nos poupou trabalho, pois a página de alteração seria um simples CTRL-C, CTRL+V do que ele havia feito.
[/quote]

Vc só pode estar brincando. Vc prefere copy-paste a centralização ? A explicação de que vc trabalhou com mainframe explica muito, mas não isso. Copy-paste é coisa de amador.

O seu colega tem razão. A tela de adição e edição devem ser a mesma. Eu disse a “tela”, ou seja , a JSP. A URL pode ser outra, e os controladores podem ser outros, mas em geral a regra é simples : se tem id, atualiza, se não tem, adiciona.

Isto é um conceito comum e usado constantemente , é quase um padrão. O sistema oferece algum tipo de listagem para procurar as instancias da entidade, da listagem vc pode adicionar uma nova ou editar uma das que listou. Vai para a mesma página , passa pelos mesmos controladores e a unica diferença é a presença ou ausencia do Id.

O seu problema é o nome “cadastrar” que é péssimo. A pagina se deve chamar edição. Toda a manipulação é uma edição, inclusive a criação. A separação só faz sentido quando a adição e a edição são conceptualmente diferentes. Por exemplo, a adição é feita com um wizard e a edição é feita em cima de um formulario com todos os campos. Lembre-se que pessoas com credenciais de segurança diferentes podem acessar uma e não a outra.

Conceptualmente o seu socio está certo e vc está errado. O vosso problema é com a implementação. E isso tlv se deva a uma fraca abstração ou imaturidade da vossa parte. Procurem seguir boas práticas e nao ver adição e edição como coisas diferentes. Elas podem ser diferentes para o usuário , mas não o são para o programador.

[bronx]

Olá sergiotaborda!

Obrigado pela participação.

Mas cara, do jeito que você colocou as informações, leva a entender que só existe uma maneira correta.
Não disse que prefiro o “pattern” copy & paste rs. Prefiro desacoplamento.

Sou a favor da nítida separação de papéis e obrigações. Concordo que a JSP possa ser a mesma, mas não concordo quando diz a mesma coisa em relação aos métodos. “editar” é um nome vago, e atrapalha na documentação do sistema. Um novo membro na equipe não saberia logo de cara do que se trata o método. Teria que gastar um tempo lendo o código para entender sua real função. Sendo assim, conceitualmente, essa abordagem não seria a mais adequada (não direi “errada”).

Outra coisa a se considerar é que o VRaptor só vai chamar o método de alteração caso a URI requisitada seja /cadastrar/id. Se for algo diferente disso, e não estiver mapeado nos controladores, ganho um belo 404 na tela.

E sim: alterar e inserir são ações distintas.

No nosso caso especificamente, existem detalhes que devem ser levados em conta durante a alteração, mas que podem ser ignorados durante a inserção. Não dá para tratar ambas operações como sendo a mesma.

De qualquer forma, valeu. Agregou ao tópico.

Mais alguém afim de opinar?

Abs

[Leonardo3001]

Assim, não existe nenhuma regra que diz: “para cada controller uma view”. Eu faria assim: no controller, haveria os métodos de editar e adicionar, mas ambos apontariam para a mesma página (um dos métodos daria um forward explícito).

Aí, caso as páginas editar e adicionar diferissem um pouquinho, então você separaria a parte comum num fragmento (*.jspf), e duas páginas (adicionar.jsp e editar.jsp) o referenciaria.

E, se as páginas se diferissem completamente, você criaria duas páginas separadas mesmo.

O impressionante é que é teu sócio. Teoricamente, não há hierarquia. Você não precisa ficar numa posição de vendido, precisando você explicar que tem que ser feito assim e assado; e que, caso contrário, é do jeito dele por default.

Dá um soco nele e faz o que você achar melhor.

[bronx]

Hahahaha!

Quisera eu que fosse simples resolver os “deadlocks”.

Não aceitei a parada do jeito que foi proposto. Tanto que alterei o método, e isso que gerou toda a “discução”. ^^
Mas temos que buscar argumentos para nossas colocações. Se eu acho que a coisa deve ser feita de outra maneira, tenho que no mínimo explanar o porquê.

Acho que vamos deixar assim mesmo: mesma JSP, actions diferentes.

Sintam-se a vontade para extender a discução.

[]'s

[sergiotaborda]

[quote=bronx]Olá sergiotaborda!

Obrigado pela participação.

Mas cara, do jeito que você colocou as informações, leva a entender que só existe uma maneira correta.
Não disse que prefiro o “pattern” copy & paste rs. Prefiro desacoplamento.

Sou a favor da nítida separação de papéis e obrigações. Concordo que a JSP possa ser a mesma, mas não concordo quando diz a mesma coisa em relação aos métodos. “editar” é um nome vago, e atrapalha na documentação do sistema. Um novo membro na equipe não saberia logo de cara do que se trata o método. Teria que gastar um tempo lendo o código para entender sua real função. Sendo assim, conceitualmente, essa abordagem não seria a mais adequada (não direi “errada”).
[/quote]

Acho que vc não leu atentamente. Eu disse que só deveria haver uma JSP e que os métodos poderiam ser iguais ou não. E falei que, em geral, são realmente o mesmo. Editar é menos vago que cadastrar. Métodos não estão na documentação, são a documentação.

Muito simples: separar coisas que deveriam ir juntas é repetição. mantenha o seu sistema seco (DRY - Don’t repeat yourself)

Se vc não está convencido pelos argumentos faça de um jeito diferente do seu colega. Depois, daqui a uns meses quando vc não lembrar mais que alterado A, tem que alterar B o seu codigo vai quebrar o dele não.

Outra coisa a se considerar é que o VRaptor só vai chamar o método de alteração caso a URI requisitada seja /cadastrar/id. Se for algo diferente disso, e não estiver mapeado nos controladores, ganho um belo 404 na tela.

Isso é problema seu. Foi vc que escolher esse framework e foi vc que decidiu usar REST e foi vc que decidiu que o URL seria esse.
O ponto aqui é se deve haver duas jsp e dois controladores, não se o VRaptor é flexivel ou não.

Só deve existir uma JSP (regra geral) e não ha necessidade de ter dois controladores.

Veja, o usuario aperta “novo” vc vai no método “novo” , prepara o formulário (carregando combos e essas coisas) devolve para a pagina de edição. O cada dá salvar. Vai para o método salvar. Lê o formulário copia para um bean e salva. Agora o cara escolhe editar, vc manda para o método editar com o id, o método carrega o bean do banco, carrega combos e outros dados de suporte e renderiza a mesma jsp.

GET URL novo : cliente/novo
GET URL editar : cliente/editar/${id}
POST URL salvar cliente/salvar

E sim: alterar e inserir são ações distintas.

Acções, não algoritmos.

No nosso caso especificamente, existem detalhes que devem ser levados em conta durante a alteração, mas que podem ser ignorados durante a inserção. Não dá para tratar ambas operações como sendo a mesma.

Isso é uma má desculpa.

public insertOrEdit(Cliente cliente){

    if(isNew(cliente)){
          // do things that only are done to new instances
          // example : 

          cliente.setId(Sequenciator.sequenceFor(Cliente.class).next());
   }

   // do things that are done to all instances.
}

DRY, é só o que posso dizer.

[bronx]

sergiotaborda, entendi seu ponto de vista.

Em momento algum contestei a flexibilidade do VRaptor. Só o citei para mostrar que tinha mais um problema com a abordagem adotada.

O que estava rolando era que até o GET que vc descreveu estava sendo o mesmo para ambas ações.