Problema no estilo de codificação: Devemos ter funções que pegam um parâmetro, modificam-no e então RETURN esse parâmetro?

Estou tendo um pouco de debate com meu amigo sobre se essas duas práticas são apenas dois lados da mesma moeda ou se uma é realmente melhor.

Temos uma função que pega um parâmetro, preenche um membro dele e o retorna:

Item predictPrice(Item item)

Acredito que, como ele funciona no mesmo objeto que é passado, não faz sentido retornar o item. De fato, se alguma coisa, na perspectiva do chamador, ele confunde os assuntos, pois você poderia esperar que ele retornasse um novo item, o que não acontece.

Ele afirma que isso não faz diferença, e nem importaria se ele criou um novo item e o devolveu. Discordo totalmente, pelos seguintes motivos:

• Se você tiver várias referências ao item que passa (ou ponteiros ou o que for), alocar um novo objeto e devolvê-lo é de importância material, pois essas referências estarão incorretas.

• Em linguagens gerenciadas sem memória, a função que aloca uma nova instância reivindica a propriedade da memória e, portanto, teríamos que implementar um método de limpeza chamado em algum momento.

• Alocar na pilha é potencialmente caro e, portanto, é importante se a função chamada faz isso.

Portanto, acredito que é muito importante poder ver através de uma assinatura de métodos se ele modifica um objeto ou aloca um novo. Como resultado, acredito que, como a função apenas modifica o objeto passado, a assinatura deve ser:

void predictPrice(Item item)

Em todas as bases de código (reconhecidamente bases de código C e C ++, e não Java, que é a linguagem em que estamos trabalhando) com as quais trabalhei, o estilo acima foi essencialmente respeitado e mantido por programadores consideravelmente mais experientes. Ele afirma que, como o tamanho da minha amostra de bases de código e colegas é pequeno em relação a todas as bases de código e colegas possíveis, minha experiência não é um verdadeiro indicador sobre se alguém é superior.

Então, algum pensamento?

Isso realmente é uma questão de opinião, mas para o que vale a pena, acho enganoso retornar o item modificado se o item for modificado no local. Separadamente, se predictPricefor para modificar o item, ele deverá ter um nome que indique que fará isso (como setPredictPricealgo assim).

Eu preferiria (em ordem)

1. Esse predictPriceera um método deItem (módulo uma boa razão para não ser, o que poderia muito bem haver no seu design geral) que

   • Devolveu o preço previsto ou

   • Tinha um nome como em setPredictedPricevez disso

2. Isso predictPrice não foi modificado Item, mas retornou o preço previsto

3. Esse predictPricefoi um voidmétodo chamadosetPredictedPrice

4. Isso predictPriceretornou this(para o encadeamento de métodos para outros métodos em qualquer instância da qual faz parte) (e foi chamado setPredictedPrice)

Dentro de um paradigma de design orientado a objetos, as coisas não devem modificar objetos fora do próprio objeto. Quaisquer alterações no estado de um objeto devem ser feitas através de métodos no objeto.

Portanto, void predictPrice(Item item) como uma função membro de alguma outra classe está errada . Poderia ter sido aceitável nos dias de C, mas para Java e C ++, as modificações de um objeto implicam um acoplamento mais profundo ao objeto que provavelmente levaria a outros problemas de design no futuro (quando você refatorar a classe e alterar seus campos, agora que 'predictPrice' precisa ser alterado para em outro arquivo.

Retornando um novo objeto, não tem efeitos colaterais associados, o parâmetro transmitido não é alterado. Você (o método predictPrice) não sabe onde mais esse parâmetro é usado. ÉItem a chave para um hash em algum lugar? você mudou o código de hash para fazer isso? Alguém mais está esperando que não mude?

Esses problemas de design sugerem fortemente que você não deve modificar objetos (em muitos casos, eu recomendaria a imutabilidade) e, se o fizer, as alterações no estado devem ser contidas e controladas pelo próprio objeto, em vez de algo mais fora da classe.

Vamos ver o que acontece se alguém mexer com os campos de algo em um hash. Vamos pegar um código:

import java.util.*;

public class Main {
    public static void main (String[] args) {
        Set set = new HashSet();
        Data d = new Data(1,"foo");
        set.add(d);
        set.add(new Data(2,"bar"));
        System.out.println(set.contains(d));
        d.field1 = 2;
        System.out.println(set.contains(d));
    }

    public static class Data {
        int field1;
        String field2;

        Data(int f1, String f2) {
            field1 = f1;
            field2 = f2;
        }

        public int hashCode() {
            return field2.hashCode() + field1;
        }

        public boolean equals(Object o) {
            if(!(o instanceof Data)) return false;
            Data od = (Data)o;
            return od.field1 == this.field1 && od.field2.equals(this.field2);

        }
    }
}

^ideona

E admito que esse não é o melhor código (acessando diretamente os campos), mas serve para demonstrar um problema com dados mutáveis ​​sendo usados ​​como chave para um HashMap ou, neste caso, apenas sendo colocados em um HashSet.

A saída deste código é:

true
false

O que aconteceu é que o hashCode usado quando foi inserido é onde o objeto está no hash. Alterar os valores usados ​​para calcular o hashCode não recalcula o próprio hash. Esse é um perigo de colocar qualquer objeto mutável como chave para um hash.

Assim, voltando ao aspecto original da pergunta, o método chamado não "sabe" como o objeto que está recebendo como parâmetro está sendo usado. Fornecer o "lets mutate the object" como a única maneira de fazer isso significa que há uma série de erros sutis que podem surgir. _{^{Como perder valores em um hash ... a menos que você adicione mais e o hash seja revisado novamente - confie em mim , isso é um bug desagradável para caçar (perdi o valor até adicionar mais 20 itens ao hashMap e, de repente, ele aparece novamente).}}

• A menos que haja uma boa razão para modificar o objeto, retornar um novo objeto é a coisa mais segura a se fazer.
• Quando não é uma boa razão para modificar o objecto, que a modificação deve ser feito pelo próprio (métodos invocando) objecto em vez de através de uma função externa que pode mexer os campos.
  • Isso permite que o objeto seja refatorado com um custo de manutenção menor.
  • Isso permite que o objeto verifique se os valores que calculam seu código de hash não mudam (ou não fazem parte do seu cálculo de hashcode)

Relacionado: Sobrescrevendo e retornando o valor do argumento usado como condicional de uma instrução if, dentro da mesma instrução if

Eu sempre sigo a prática de não alterar o objeto de outra pessoa. Ou seja, apenas objetos mutantes pertencentes à classe / struct / whathaveyou que você está dentro.

Dada a seguinte classe de dados:

class Item {
    public double price = 0;
}

Isso é bom:

class Foo {
    private Item bar = new Item();

    public void predictPrice() {
        bar.price = bar.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
foo.predictPrice();
System.out.println(foo.bar.price);
// Since I called predictPrice on Foo, which takes and returns nothing, it's
// apparent something might've changed

E isso é muito claro:

class Foo {
    private Item bar = new Item();
}
class Baz {
    public double predictPrice(Item item) {
        return item.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
Baz baz = Baz();
foo.bar.price = baz.predictPrice(foo.bar);
System.out.println(foo.bar.price);
// Since I explicitly set foo.bar.price to the return value of predictPrice, it's
// obvious foo.bar.price might've changed

Mas isso é muito enlameado e misterioso para o meu gosto:

class Foo {
    private Item bar = new Item();
}
class Baz {
    public void predictPrice(Item item) {
        item.price = item.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
Baz baz = Baz();
baz.predictPrice(foo.bar);
System.out.println(foo.bar.price);
// In my head, it doesn't appear that to foo, bar, or price changed. It seems to
// me that, if anything, I've placed a predicted price into baz that's based on
// the value of foo.bar.price

A sintaxe é diferente entre idiomas diferentes e não faz sentido mostrar um pedaço de código que não é específico a um idioma, porque significa coisas completamente diferentes em idiomas diferentes.

Em Java, Itemé um tipo de referência - é o tipo de ponteiros para objetos que são instâncias Item. No C ++, esse tipo é escrito como Item *(a sintaxe da mesma coisa é diferente entre os idiomas). Portanto, Item predictPrice(Item item)em Java é equivalente a Item *predictPrice(Item *item)C ++. Nos dois casos, é uma função (ou método) que leva um ponteiro para um objeto e retorna um ponteiro para um objeto.

Em C ++, Item(sem mais nada) é um tipo de objeto (assumindo que Itemé o nome de uma classe). Um valor desse tipo "é" um objeto e Item predictPrice(Item item)declara uma função que pega um objeto e retorna um objeto. Como &não é usado, é passado e retornado por valor. Isso significa que o objeto é copiado quando passado e copiado quando retornado. Não há equivalente em Java porque Java não possui tipos de objetos.

Então qual é? Muitas das coisas que você está perguntando na pergunta (por exemplo, "eu acredito que, como funciona no mesmo objeto que é passado em ...") depende de entender exatamente o que está sendo passado e devolvido aqui.

A convenção que vi bases de código aderir é preferir um estilo que é claro a partir da sintaxe. Se a função mudar de valor, prefira passar pelo ponteiro

void predictPrice(Item * const item);

Esse estilo resulta em:

• Chamando você vê:

  predictPrice (& meu_item);

e você sabe que será modificado pela sua aderência ao estilo.

• Caso contrário, prefira passar por const ref ou value quando você não quiser que a função modifique a instância.

Embora eu não tenha uma preferência forte, eu votaria que o retorno do objeto leva a uma melhor flexibilidade para uma API.

Observe que ele só faz sentido enquanto o método tem liberdade para retornar outro objeto. Se o seu javadoc diz @returns the same value of parameter a, é inútil. Mas se o seu javadoc diz @return an instance that holds the same data than parameter a, retornar a mesma instância ou outra é um detalhe de implementação.

Por exemplo, no meu projeto atual (Java EE), tenho uma camada de negócios; para armazenar no banco de dados (e atribuir um ID automático) a um funcionário, digamos, a um funcionário que tenha algo como

 public Employee createEmployee(Employee employeeData) {
   this.entityManager.persist(employeeData);
   return this.employeeData;
 }

Obviamente, não há diferença com essa implementação porque o JPA retorna o mesmo objeto depois de atribuir o ID. Agora, se eu mudei para JDBC

 public Employee createEmployee(Employee employeeData) {
   PreparedStatement ps = this.connection.prepareStatement("INSERT INTO EMPLOYEE(name, surname, data) VALUES(?, ?, ?)");
   ... // set parameters
   ps.execute();
   int id = getIdFromGeneratedKeys(ps);
   ??????
 }

Agora em ????? Eu tenho três opções.

1. Defina um setter para Id e retornarei o mesmo objeto. Feio, porque setIdestará disponível em qualquer lugar.

2. Faça um trabalho pesado de reflexão e defina o ID no Employeeobjeto. Sujo, mas pelo menos é limitado ao createEmployeeregistro.

3. Forneça um construtor que copie o original Employeee aceite também o idconjunto.

4. Recupere o objeto do banco de dados (talvez necessário se alguns valores de campo forem calculados no banco de dados ou se você desejar preencher um realtion).

Agora, para 1. e 2. você pode estar retornando a mesma instância, mas para 3. e 4. você estará retornando novas instâncias. Se, a partir do princípio que você estabeleceu em sua API, o valor "real" será o valor retornado pelo método, você terá mais liberdade.

O único argumento real em que posso pensar é que, usando as implementações 1. ou 2., as pessoas que usam sua API podem ignorar a atribuição do resultado e seu código seria interrompido se você mudar para implementações 3. ou 4.).

De qualquer forma, como você pode ver, minha preferência é baseada em outras preferências pessoais (como proibir um levantador de IDs), então talvez isso não se aplique a todos.

Ao codificar em Java, deve-se tentar fazer com que o uso de cada objeto do tipo mutável corresponda a um dos dois padrões:

1. Exatamente uma entidade é considerada o "proprietário" do objeto mutável e considera o estado desse objeto como parte do seu próprio. Outras entidades podem conter referências, mas devem considerar a referência como identificando um objeto que pertence a outra pessoa.

2. Apesar de o objeto ser mutável, ninguém que possua uma referência a ele pode modificá-lo, tornando a instância efetivamente imutável (observe que, devido a peculiaridades no modelo de memória do Java, não há uma boa maneira de fazer com que um objeto imutável efetivamente tenha threads. semântica imutável segura sem adicionar um nível extra de indireção a cada acesso). Qualquer entidade que encapsula o estado usando uma referência a esse objeto e deseja alterar o estado encapsulado deve criar um novo objeto que contenha o estado apropriado.

Eu não gosto de ter métodos que modifiquem o objeto subjacente e retornem uma referência, pois eles dão a aparência de ajustar o segundo padrão acima. Existem alguns casos em que a abordagem pode ser útil, mas o código que vai mudar os objetos deve parecer que vai fazê-lo; código comomyThing = myThing.xyz(q); parece muito menos com o objeto identificado pelo myThingque seria simplesmente myThing.xyz(q);.