La progettazione di un metodo che modifica l'argomento (se era oggetto) rappresenta una buona pratica?

Se hai un metodo del genere, allora tale metodo dovrebbe in realtà essere parte della classe Player . Questo quindi indica chiaramente che il metodo potrebbe cambiare lo stato dell'oggetto.

class Player
{
    void AddLevelBy1()
    {
        this.level += 1;
    }
}

player.AddLevelBy1()

I problemi iniziano quando hai cambiato più di 1 oggetti, ma non c'è una soluzione generale a questo.

If using c#, the "out" keyword can be used to indicate the change.

Questo è sbagliato. La parola chiave out indica qualcosa di completamente diverso. Vedi di più qui: link

Sebbene il tuo esempio sia semplicistico e possa essere modellato in modo diverso (come suggerito da @Euphoric o usando metodi di estensione se la classe non è nella tua base di codice), ci sono molti casi d'uso in cui è legittimo che un metodo modifichi lo stato dei loro parametri.

Ad esempio, i processi di builder, i tuoi algoritmi ricorsivi, come cicli di rilevamento DFS .

Puoi usare Law of Demeter come regola empirica di cosa dovrebbe fare o non fare un metodo:

• You can play with yourself.
• You can play with your own toys (but you can’t take them apart),
• You can play with toys that were given to you.
• And you can play with toys you’ve made yourself.

Ovviamente, quando un metodo ha effetti collaterali, dovrebbe essere dichiarato nel suo nome: IncrementPlayerLevel(Player player) dichiara chiaramente che cambierà il livello del giocatore ...

Dall'altro lato delle considerazioni sulla progettazione, a volte è fondamentale che i metodi non cambino lo stato. Ad esempio, implementando un Actor model per i calcoli paralleli. Considerando questo, è meglio progettare gli oggetti di stato come immutable , quindi non devi chiederti se qualche metodo ha cambiato i tuoi oggetti senza che tu ne sia a conoscenza.

Come menzionato nella risposta di Uri , idealmente i tuoi tipi sarebbero immutabili. La mutabilità complica i programmi. Ma le lingue tradizionali hanno scarso supporto per l'immutabilità, e abbiamo una lunga storia di mutazione che risale ai giorni di assemblea, quindi a volte non si può andare avanti con la mutabilità. A volte la mutazione di un argomento è inevitabile - ad esempio, se la classe non è sotto il tuo controllo e non fornisce alcun modo di clonazione (cioè la copia profonda) dei suoi valori. Quindi, potrebbe non essere una buona pratica, ma a volte non è possibile aggirarla.

Alcuni hanno suggerito di rendere il mutatore un metodo della classe. Anche se la classe è sotto il tuo controllo, questo non ha necessariamente senso: c'è un numero infinito di cose che potresti voler fare con la tua classe mutevole, e quindi un numero infinito di funzioni che potrebbero aver bisogno di mutarlo. In secondo luogo, non tutto è un metodo. Se si ha un'operazione commutativa, non esiste un "destinatario" per la funzione; entrambi gli argomenti sono ugualmente validi. Se hai una funzione che funziona su argomenti di due tipi diversi, perché la funzione dovrebbe appartenere a una classe o l'altra? A volte questo non ha senso. Se non hai bisogno di accedere ai membri privati della classe, non ha senso rendere la funzione un metodo.

Ok, quindi è necessario modificare l'argomento. Cosa puoi fare per mitigare il danno potenziale? Come hai sottolineato, è fondamentale che il nome della funzione ti informi in modo inequivocabile che ciò accadrà. In secondo luogo, dovresti evitare di creare funzioni accessorie e mutanti allo stesso tempo . Se lo scopo di una funzione è di restituire alcune informazioni su un oggetto, le persone si sorprenderanno se muta anche quell'oggetto o qualsiasi altro.

Uri espone la Legge di Demetra, che è un'eccellente linea guida, ma tieni presente che è rilevante solo per le astrazioni. L'idea è che un'astrazione dovrebbe nascondere la sua implementazione, quindi catene come foo.bar.baz() sono di solito un segno che non stai facendo un ottimo lavoro lì. Ma non tutto è un'astrazione! Alcuni tipi (ad esempio tuple, record) sono semplicemente contenitori di dati e l'accesso ai loro membri non costituisce una violazione della legge, anche se il risultato finale è ancora una catena tratteggiata. (Naturalmente, se la tua astrazione non dovesse esporre quella tupla in primo luogo, è ancora un problema.)