I metodi che modificano i parametri del tipo di riferimento sono errati?

Question

Ho visto metodi come questo:

public void Foo(List<string> list)
{
    list.Add("Bar");
}

È questa buona pratica modificare i parametri in un metodo?

Non sarebbe meglio?

public List<string> Foo(List<string> list)
{
    // Edit
    List<string> newlist = new List<string>(list);
    newlist.Add("Bar");
    return newlist;
}

Sembra proprio che il primo esempio abbia effetti collaterali inaspettati.

Answer 1

Nell'esempio che hai dato, il primo mi sembra molto più bello del secondo. Se vedessi un metodo che accettava un elenco e restituiva anche un elenco, la mia prima ipotesi sarebbe che stesse restituendo un nuovo elenco e non toccando quello che gli era stato dato. Il secondo metodo, quindi, è quello con effetti collaterali inaspettati.

Fintanto che i metodi vengono nominati in modo appropriato, c'è poco pericolo nel modificare il parametro. Considera questo:

public void Fill<T>(IList<T> list)
{
    // add a bunch of items to list
}

Con un nome come " Fill " puoi essere abbastanza sicuro che il metodo modificherà l'elenco.

Answer 2

Francamente, in questo caso, entrambi i metodi fanno più o meno la stessa cosa. Entrambi modificheranno l ' Elenco che è stato passato.

Se l'obiettivo è avere liste immutabili con tale metodo, il secondo esempio dovrebbe creare una copia dell'elenco inviato, quindi eseguire Aggiungi operazione sul nuovo Elenco e quindi restituirlo.

Non ho familiarità con C # né .NET, quindi la mia ipotesi sarebbe qualcosa sulla falsariga di:

public List<string> Foo(List<string> list)
{
    List<string> newList = (List<string>)list.Clone();
    newList.Add("Bar");
    return newList;
}

In questo modo, il metodo che chiama il metodo Foo restituirà il Elenco appena creato e il Elenco originale che è stato passato non sarebbe toccato.

Questo dipende davvero dal "contratto" delle tue specifiche o API, quindi nei casi in cui Elenco può essere semplicemente modificato, non vedo alcun problema con il primo approccio.

Answer 3

Stai facendo esattamente la stessa cosa in entrambi i metodi, solo uno di loro sta restituendo la stessa lista.

Dipende molto da quello che stai facendo, secondo me. Assicurati solo che la tua documentazione sia chiara su ciò che sta succedendo. Scrivi pre-condizioni e post-condizioni se ti piacciono queste cose.

Answer 4

In realtà non è così inaspettato che un metodo che accetta un elenco come parametro modifica l'elenco. Se si desidera un metodo che legge solo dall'elenco, utilizzare un'interfaccia che consenta solo la lettura:

public int GetLongest(IEnumerable<string> list) {
    int len = 0;
    foreach (string s in list) {
        len = Math.Max(len, s.Length);
    }
    return len;
}

Usando un'interfaccia come questa non solo proibisci al metodo di cambiare l'elenco, ma diventa anche più flessibile in quanto può usare qualsiasi raccolta che implementa l'interfaccia, come ad esempio un array di stringhe.

Alcune altre lingue hanno una parola chiave const che può essere applicata ai parametri per impedire a un metodo di modificarli. Poiché .NET ha interfacce che puoi usare per questo e stringhe che sono immutabili, non c'è davvero bisogno dei parametri const .

Answer 5

L'avvento dei metodi di estensione ha reso un po 'più facile gestire i metodi che introducono effetti collaterali. Ad esempio, nel tuo esempio diventa molto più intuitivo dire

public static class Extensions
{
  public static void AddBar(this List<string> list)
  {
     list.Add("Bar");
  }
}

e chiamalo con

mylist.AddBar();

che rende più chiaro che qualcosa sta accadendo all'elenco.

Come menzionato nei commenti, questo è molto utile nelle liste poiché le modifiche a una lista possono essere più confuse. Su un oggetto semplice, tenderei solo a modificare l'oggetto sul posto.