Problema dello stile di codifica: dovremmo avere delle funzioni che accettano un parametro, lo modificano e quindi RESTITUISCONO quel parametro?

Questa è davvero una questione di opinione, ma per quello che vale, trovo fuorviante restituire l'articolo modificato se l'elemento è modificato sul posto. Separatamente, se predictPrice sta per modificare l'elemento, dovrebbe avere un nome che indica che lo farà (come setPredictPrice o alcuni di questi).

Preferirei (in ordine)

1. Quella predictPrice era un metodo di Item (modulo una buona ragione per non esserlo, cosa che potrebbe esserci nel tuo progetto generale) che

   • Restituito il prezzo previsto o

   • Aveva un nome come setPredictedPrice invece

2. Quella predictPrice non modifica Item , ma ha restituito il prezzo previsto

3. Quella predictPrice era un metodo void chiamato setPredictedPrice

4. Quella predictPrice ha restituito this (per il metodo concatenato ad altri metodi su qualsiasi istanza di cui fa parte) (ed è stato chiamato setPredictedPrice )

All'interno di un paradigma di progettazione orientato agli oggetti, le cose non dovrebbero modificare gli oggetti al di fuori dell'oggetto stesso. Qualsiasi modifica allo stato di un oggetto dovrebbe essere eseguita attraverso i metodi sull'oggetto.

Quindi, void predictPrice(Item item) come funzione membro di qualche altra classe è sbagliato . Avrebbe potuto essere accettabile ai tempi di C, ma per Java e C ++, le modifiche di un oggetto implicano un accoppiamento più profondo con l'oggetto che probabilmente porterebbe ad altri problemi di progettazione lungo la strada (quando si rifatta la classe e si cambiano i suoi campi, ora che 'prevedere il prezzo' deve essere cambiato in qualche altro file.

Restituendo un nuovo oggetto, non ha effetti collaterali associati, il parametro passato non viene modificato. Tu (il metodo predictPrice) non sai dove viene usato quel parametro. Item è una chiave per un hash da qualche parte? hai cambiato il suo codice hash nel fare questo? Qualcun altro si sta aggrappando ad esso aspettandosi che non cambi?

Questi problemi di progettazione sono quelli che suggeriscono strongmente che non dovresti modificare gli oggetti (in molti casi discuterei dell'immutabilità), e se lo fai, le modifiche allo stato dovrebbero essere contenute e controllate dall'oggetto stesso piuttosto che qualcos'altro al di fuori della classe.

Vediamo cosa succede se si armeggia con i campi di qualcosa in un hash. Prendiamo un po 'di codice:

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);

        }
    }
}

^ideone

E ammetto che questo non è il codice più grande (accedere direttamente ai campi), ma serve allo scopo di dimostrare un problema con i dati mutabili usati come chiave per una HashMap, o in questo caso, solo per essere mettere in un HashSet.

L'output di questo codice è:

true
false

Quello che è successo è che l'hashCode usato quando è stato inserito è dove l'oggetto si trova nell'hash. La modifica dei valori utilizzati per calcolare l'hashCode non ricalcola l'hash stesso. Questo è un pericolo nel mettere qualsiasi oggetto mutabile come chiave per un hash.

Quindi, tornando all'aspetto originale della domanda, il metodo che viene chiamato non "sa" come viene utilizzato l'oggetto che sta ricevendo. Fornire il "lascia mutare l'oggetto" come l'unico modo per farlo significa che ci sono un certo numero di bug sottili che possono insinuarsi. _{^{Come perdere valori in un hash ... a meno che tu non aggiunga altro e il l'hash viene ridisegnato - fidati di me, questo è un bug brutto da cercare (ho perso il valore finché non aggiungo altri 20 elementi all'hashMap e poi all'improvviso si mostra di nuovo).}}

• A meno che non ci sia molto un buon motivo per modificare l'oggetto, restituire un nuovo oggetto è la cosa più sicura da fare.
• Quando è un buon motivo per modificare l'oggetto, tale modifica dovrebbe essere eseguita dall'oggetto stesso (metodi di richiamo) piuttosto che tramite una funzione esterna che può manipolare i suoi campi.
  • Ciò consente di refactoring dell'oggetto con un minore costo di manutenzione.
  • Ciò consente all'oggetto di assicurarsi che i valori che calcolano il suo codice hash non cambino (o che non facciano parte del suo calcolo dell'hashcode)

Correlati: Sovrascrittura e restituzione del valore dell'argomento utilizzato come condizionale di un'istruzione if, all'interno della stessa istruzione if

La sintassi è diversa tra lingue diverse e non ha senso mostrare un pezzo di codice che non è specifico di una lingua perché significa cose completamente diverse in lingue diverse.

In Java, Item è un tipo di riferimento - è il tipo di puntatori agli oggetti che sono istanze di Item . In C ++, questo tipo è scritto come Item * (la sintassi per la stessa cosa è diversa tra le lingue). Quindi Item predictPrice(Item item) in Java equivale a Item *predictPrice(Item *item) in C ++. In entrambi i casi, è una funzione (o metodo) che accetta un puntatore a un oggetto e restituisce un puntatore a un oggetto.

In C ++, Item (senza nient'altro) è un tipo di oggetto (supponendo che Item sia il nome di una classe). Un valore di questo tipo "è" un oggetto e Item predictPrice(Item item) dichiara una funzione che accetta un oggetto e restituisce un oggetto. Poiché & non viene utilizzato, viene passato e restituito dal valore. Ciò significa che l'oggetto viene copiato quando viene passato e copiato quando viene restituito. Non esiste un equivalente in Java perché Java non ha tipi di oggetto.

Quindi qual è? Molte delle cose che stai ponendo nella domanda (ad esempio "Credo che mentre funziona sullo stesso oggetto che è passato in ...") dipende dalla comprensione esatta di ciò che viene passato e restituito qui.

La convenzione alla quale ho visto rispettare le codebase è preferire uno stile chiaro dalla sintassi. Se la funzione cambia valore, preferisci passare per puntatore

void predictPrice(Item * const item);

Questo stile risulta in:

• Chiamandolo vedi:

  predictPrice (& my_item);

e sai che sarà modificato, dalla tua adesione allo stile.

• Altrimenti preferisci passare da const ref o value quando non vuoi che la funzione modifichi l'istanza.

Seguo sempre la pratica di non mutare l'oggetto di qualcun altro. Vale a dire, solo gli oggetti mutanti di proprietà della classe / struct / whathaveyou in cui ti trovi.

Data la seguente classe di dati:

class Item {
    public double price = 0;
}

Questo è OK:

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 questo è molto chiaro:

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

Ma questo è troppo fangoso e misterioso per i miei gusti:

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

Anche se non ho una strong preferenza, voterei che restituire l'oggetto porti a una migliore flessibilità per un'API.

Si noti che ha senso solo mentre il metodo ha la libertà di restituire altri oggetti. Se il tuo javadoc dice @returns the same value of parameter a , allora è inutile. Ma se il tuo javadoc dice @return an instance that holds the same data than parameter a , restituire la stessa istanza o un altro è un dettaglio di implementazione.

Ad esempio, nel mio attuale progetto (Java EE) ho un livello aziendale; per memorizzare in DB (e assegnare un ID automatico) un Dipendente, ad esempio, un dipendente che ho qualcosa di simile

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

Naturalmente, nessuna differenza con questa implementazione perché JPA restituisce lo stesso oggetto dopo aver assegnato l'Id. Ora, se passassi a 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);
   ??????
 }

Ora, a ????? Ho tre opzioni.

1. Definisci un setter per Id, e restituirò lo stesso oggetto. Brutto, perché setId sarà disponibile ovunque.

2. Esegui un po 'di riflessione approfondita e imposta l'id nell'oggetto Employee . Sporco, ma almeno è limitato al record createEmployee .

3. Fornisci un costruttore che copia il% originale% co_de e accetta anche Employee da impostare.

4. Recupera l'oggetto dal DB (forse necessario se alcuni valori di campo sono calcolati nel DB, o se desideri popolare una realtionship).

Ora, per 1. e 2. è possibile che si stia restituendo la stessa istanza, ma per 3. e 4. si ritorneranno nuove istanze. Se dal principio che hai stabilito nella tua API che il valore "reale" sarà quello restituito dal metodo, hai più libertà.

L'unica vera argomentazione a cui posso pensare è che, usando implementatios 1. o 2., le persone che usano la tua API potrebbero trascurare l'assegnazione del risultato e il loro codice si interromperebbe se si passa a 3. o 4. implementazioni) .

Ad ogni modo, come puoi vedere, la mia preferenza si basa su altre preferenze personali (come proibire un setter per Id), quindi forse non si applica a tutti.

Quando si scrive in Java, si dovrebbe cercare di fare in modo che l'uso di ogni oggetto di tipo mutabile corrisponda a uno di due pattern:

1. Esattamente un'entità è considerata il "proprietario" dell'oggetto mutevole e considera lo stato dell'oggetto come parte della propria. Altre entità possono contenere riferimenti, ma dovrebbero considerare il riferimento come identificare un oggetto che è di proprietà di qualcun altro.

2. Nonostante l'oggetto sia mutabile, a nessuno che ne abbia un riferimento è permesso di mutarlo, rendendo così l'istanza effettivamente immutabile (si noti che a causa di stranezze nel modello di memoria Java, non c'è un buon modo per renderlo efficace L'oggetto immutabile ha una semantica immutabile sicura per i thread senza aggiungere un livello aggiuntivo di riferimento indiretto a ciascun accesso). Qualsiasi entità che incapsula lo stato usando un riferimento a tale oggetto e vuole cambiare lo stato incapsulato deve creare un nuovo oggetto che contenga lo stato appropriato.

Non mi piace avere metodi che mutino l'oggetto sottostante e restituiscano un riferimento, dato che danno l'impressione di montare il secondo modello sopra. Ci sono alcuni casi in cui l'approccio può essere utile, ma il codice che sta per mutare oggetti dovrebbe guardare come lo farà; il codice come myThing = myThing.xyz(q); sembra molto meno come muta l'oggetto identificato da myThing rispetto a myThing.xyz(q); .