Les méthodes qui modifient les paramètres de type référence sont-elles mauvaises?

Question

J'ai vu des méthodes comme celle-ci:

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

Cette bonne pratique consiste-t-elle à modifier les paramètres dans une méthode?

Cela ne serait-il pas mieux?

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

On a l'impression que le premier exemple a des effets secondaires inattendus.

Answer 1

Dans l'exemple que vous avez donné, le premier me semble beaucoup plus agréable que le second. Si je voyais une méthode qui acceptait une liste et renvoyait également une liste, ma première hypothèse serait qu'elle renvoyait une nouvelle liste sans toucher à celle qui lui avait été fournie. La deuxième méthode est donc celle qui entraîne des effets secondaires inattendus.

Tant que vos méthodes sont nommées correctement, la modification du paramètre présente peu de danger. Considérez ceci:

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

Avec un nom tel que " Remplir " vous pouvez être certain que la méthode modifiera la liste.

Answer 2

Franchement, dans ce cas, les deux méthodes font plus ou moins la même chose. Les deux modifieront la liste qui a été transmise.

Si l'objectif est d'avoir des listes immuables avec une telle méthode, le deuxième exemple devrait faire une copie de la Liste qui a été envoyée, puis effectuer la Ajouter . opération sur la nouvelle liste puis retourne-la.

Je ne suis pas familier avec C # ni .NET, je pense donc que:

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

De cette manière, la méthode qui appelle la méthode Foo obtiendra la liste nouvellement créée , et le liste original qui a été transmis. ne serait pas touché.

Cela relève vraiment du " contrat " de vos spécifications ou de votre API, donc dans les cas où les Listes peuvent simplement être modifiées, je ne vois pas de problème à utiliser la première approche.

Answer 3

Vous faites exactement la même chose dans les deux méthodes, une seule renvoie la même liste.

Cela dépend vraiment de ce que vous faites, à mon avis. Assurez-vous simplement que votre documentation est claire sur ce qui se passe. Rédigez des conditions préalables et postérieures si vous aimez ce genre de chose.

Answer 4

Il n’est pas surprenant qu’une méthode prenant une liste en tant que paramètre modifie la liste. Si vous voulez une méthode qui ne lit que dans la liste, vous utiliserez une interface qui ne permet que de lire:

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

En utilisant une interface comme celle-ci, vous n'empêchez pas seulement la méthode de modifier la liste, elle devient également plus flexible car elle peut utiliser n'importe quelle collection qui implémente l'interface, comme un tableau de chaînes par exemple.

Certaines autres langues ont un mot clé const qui peut être appliqué aux paramètres pour empêcher une méthode de les modifier. Comme .NET a des interfaces que vous pouvez utiliser pour cela et des chaînes qui sont immuables, les paramètres const ne sont pas vraiment nécessaires.

Answer 5

L'avènement des méthodes d'extension a rendu un peu plus facile le traitement des méthodes qui introduisent des effets secondaires. Par exemple, dans votre exemple, il devient beaucoup plus intuitif de dire

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

et appelez-le avec

mylist.AddBar();

ce qui indique plus clairement qu’il se passe quelque chose dans la liste.

Comme indiqué dans les commentaires, ceci est particulièrement utile pour les listes car les modifications apportées à une liste peuvent être plus déroutantes. Sur un objet simple, j'aurais tendance à simplement modifier l'objet en place.