Os construtores devem ser usados apenas para efeitos colaterais?

4

Resumo: Por que é errado projetar um construtor apenas para seus efeitos colaterais e, em seguida, usar o construtor sem nunca atribuir seu valor de retorno a uma variável?

Estou trabalhando em um projeto que envolve a modelagem de jogos de cartas. A API pública do jogo usa um padrão ímpar para instanciar os objetos Rank e Suit. Ambos usam uma lista estática para rastrear instâncias. Em um baralho padrão, haverá sempre quatro objetos Suit e treze Rank. Ao construir um cartão, Ranks e Suits são obtidos com% estáticogetRank() e getSuit() , que retorna o item identificado da lista estática. Rank e Suit possuem outros métodos estáticos, como removeRank() e clear() .

O que é estranho é que não há nenhum método estático add() para adicionar uma nova classificação ou adequação à lista estática. Em vez disso, esse comportamento ocorre somente quando um construtor de classificação ou adequação é chamado por meio de new . Como as instâncias de Rank e Suit são obtidas com o getRank() estático, o valor de retorno dos novos construtores é completamente irrelevante.

Algum exemplo de código pode se parecer com isso

//set up the standard suits
Suit.clear(); //remove all Suit instances from the static list
new Suit("clubs", 0);  //add a new instance, throw away the returned value
new Suit("diamonds", 1);
new Suit("hearts", 2);
new Suit("spades", 3);

//create a card
Deck theDeck = new Deck()
for (int i = 0; i < 4; i++) {
    for (int j = 0; j < 13; j++) {
        Suit theSuit = Suit.getSuit(i);
        Rank theRank = Rank.getRank(j);
        theDeck.add(new Card(theSuit, theRank));
    }
}

Eu nunca vi código que não atribuiu o objeto sendo retornado por um construtor. Eu nem pensei que isso fosse possível em Java até que eu testei. No entanto, quando o problema foi levantado de que o método estático addSuit() estava ausente, a resposta foi

"Objects of type Suit and Rank are created via constructors. The intent is to mimic the behavior of the java.lang.Enum class in which objects are created via constructors and the class maintains the collection of such objects. Current behavior is designed as intended."

Usar construtores dessa maneira cheira muito mal para mim e quero reabrir o problema, mas não consigo encontrar muito material de suporte (diretrizes de codificação, antipadrões, outra documentação). Isso é de fato um padrão de codificação válido que eu não encontrei antes? Se não, qual é o bom argumento que posso fazer para mudar a API?

Aqui está um artigo que fala sobre o que os construtores devem fazer:

We can agree that a constructor always needs to initialize new instances, and we might even agree by now that a constructor always needs initialize instances completely.

Edita: Adicionadas informações adicionais sobre a resposta do designer ao problema. Foram adicionados alguns artigos que falam sobre o que os construtores devem e não devem fazer.

    
por BrianHVB 04.03.2017 / 05:30
fonte

5 respostas

11

Claro, tecnicamente isso pode funcionar, mas viola o chamado Princípio de menor espanto , porque a convenção típica como construtores são usados e o que eles servem é bem diferente. Além disso, viola o SRP , uma vez que agora os construtores fazem duas coisas em vez do que normalmente deveriam fazer - construindo o objeto e adicionando-o a uma lista estática.

Em suma, este é um exemplo de código sofisticado, mas não de código bom.

    
por 04.03.2017 / 09:32
fonte
6

Não vejo razão razoável para esse "padrão". Como você disse, a funcionalidade desejada seria melhor implementada por uma função estática; Embora seja possivelmente / provavelmente verdade que usar um construtor para fazer isso não produza problemas funcionais, ele quebra o Princípio de menor surpresa: como você, eu vejo esse código e penso "Huh !? O que está acontecendo aqui então? ". Eu não vejo nenhuma vantagem para fazer desta forma por ter uma função estática.

Recuando um pouco mais, a natureza global dos fatos também é um cheiro de código: o que acontece se você quiser modelar bridge e (digamos) tarot, com seus diferentes trajes no mesmo programa? Um design melhor envolveria SuitCollection ou similar.

O que você pode fazer sobre isso? Isso depende muito do contexto. Se este for um projeto comercial, fale com seus colegas para ver se há uma boa razão para essa decisão e, se não, vá e fale com quem quer que tenha essa resposta em pessoa e tente e discuta as coisas. Se for um projeto de código aberto ou similar, meu conselho pessoal seria "fugir" - código de baixa qualidade combinado com comunicações ruins não resultará em um projeto divertido.

    
por 04.03.2017 / 08:14
fonte
2
Em primeiro lugar, digamos que um dos principais objetivos do design de software é a manutenção - garantir que o código seja fácil de entender e modificar.

Em seguida, digamos que o construtor é um tipo especial de método que inicializa um objeto recém-criado para um estado válido.

À luz dessas duas suposições, é seguro dizer que é inválido ter um construtor de efeitos colaterais em um design adequado. O construtor, por sua natureza, não deve ter nenhum efeito além do objeto recém-criado e de quaisquer objetos que ele inclua.

Dado este código:

public void MyMethod()
{
    DoSomething();
    new MyClass();
    DoSomething();
}

Você acharia fácil manter? Você pode reordenar MyClass() e DoSomething() ? Você não sabe até visitar o construtor MyClass e ver o que ele faz, certo?

Dessa forma, você logo terá que entender todo o programa para modificar pequenas partes dele. Este é exatamente o oposto do bom design.

Além disso, ter um construtor de efeitos colaterais geralmente implica em algo estático acontecendo em segundo plano, que é novamente uma bandeira vermelha quando se trata de testes e design adequado. Embora aparentemente barato e gerenciável, o contexto estático cresce muito rapidamente e dificulta futuras modificações no programa.

Obviamente, a programação é um trabalho de compromisso e se a presença de coleta estática de tipos de cartões obtiver grandes melhorias no time-to-market (tenho dúvidas muito strongs sobre isso), ainda seria muito melhor ver a intenção expressa através de métodos estáticos e inicialização única:

public void ApplicationSetup()
{
    Suit.Initialize("clubs", "diamonds", "hearts", "spades"); // throws if called more than once
}

No entanto, mesmo assim, você pode acabar chamando partes do programa que exigem que Suit seja inicializado antes que a inicialização realmente ocorra. A melhor maneira de expressar "esta classe / método requer que Suit seja inicializado" é usar Suit como parâmetro. O que se opõe a toda a idéia estática de Suit .

    
por 04.03.2017 / 09:19
fonte
1

Não! É um código ruim! É um padrão confuso que não oferece nenhum benefício comparado a fazê-lo de maneira direta.

O construtor é usado apenas como um método estático sofisticado. Chamá-lo como Suit.addSuit("spades", 3); teria o mesmo efeito sem ser confuso e retornar uma instância inútil.

A comparação com Enum não justifica o uso estranho de construtores - os enums não usam esse padrão. Com enums, os construtores são usados para criar valores do tipo (por exemplo, como alternativa para Suit.getSuit() ), não para definir as opções possíveis, que acontecem por meio de uma sintaxe dedicada.

Além disso, o uso de Suit.Clear() indica que isso não é equivalente a Enums. O ponto de enums é que os valores disponíveis são definidos e fixados em tempo de compilação e, portanto, podem ser verificados estaticamente. O método Clear() indica que as opções irão variar no tempo de execução. Se você tiver conjuntos diferentes de ternos em tempo de execução, ele não deve ser definido como uma coleção estática.

    
por 04.03.2017 / 10:17
fonte
1

Concordo com as outras respostas de que esse padrão é ruim e viola vários princípios de engenharia de software. Aparentemente, seus colegas de trabalho não se importam muito com eles. O que você precisa são exemplos de casos de uso em que esse padrão falha ou é difícil de usar. Aqui está um casal:

  1. O jogo é Bridge. Após a licitação, às vezes você deve alterar o nome de uma das quatro ações existentes para "trunfo". Tente escrever esse código. Eu prevejo que será nojento, com bugs, e pode até quebrar o código existente.
  2. O jogo é Hearts. Renomeie a Rainha de Copas para "A Dama". (Versão PG).
  3. Seu jogo é um grande sucesso na China e na Armênia. Adicione um menu para o idioma que eles possam escolher a qualquer momento durante o jogo. Você deve levar esta versão ao mercado antes que seus concorrentes o façam! Ah, sim, é um jogo multiplayer e a grande disputa entre Armênia e Xangai está chegando, com uma audiência de TV a cabo nos EUA.
por 04.03.2017 / 17:25
fonte