Codage question de style: Faut-il avoir des fonctions qui prennent un paramètre, le modifier, puis retournez ce paramètre?

Je vais avoir un peu d'un débat avec mon ami quant à savoir si ces deux pratiques ne sont que les deux faces d'une même pièce, ou si l'on est réellement mieux.

Nous avons une fonction qui prend un paramètre, remplir un membre, puis retourne:

Item predictPrice(Item item)

Je crois que cela fonctionne sur le même objet qui est transmis, il n'y a aucune raison de passer à renvoyer l'article.

Selon lui, il ne fait aucune différence, il serait même pas d'importance si elle créait un nouvel article et le retourner. Je suis fortement en désaccord, pour les raisons suivantes:

• Si vous avez plusieurs références à l'élément passé (ou pointeurs ou autre), il alloue un nouvel objet et le renvoie est d'une importance matérielle car ces références seront incorrectes.

• Dans les langages non gérés en mémoire, la fonction allouant une nouvelle instance revendique la propriété de la mémoire, et donc nous devrions implémenter une méthode de nettoyage qui est appelée à un moment donné.

• L'allocation sur le tas est potentiellement coûteux, il est donc important de savoir si la fonction n'appelé que.

Par conséquent, je crois qu'il est très important d'être en mesure de voir par une signature de méthodes si elle modifie un objet, ou alloue un nouveau. Par conséquent, je crois que la fonction ne fait que modifier l'objet adoptée en, la signature doit être:

void predictPrice(Item item)

Dans tous les codebase (il est vrai C et de C codebases de non Java qui est la langue que nous travaillons) Je travaille avec le style ci-dessus a essentiellement été respecté et maintenu par des programmeurs beaucoup plus expérimentés. Il prétend que ma taille de l'échantillon et codebases collègues est petit de tous les possibles et codebases collègues, et donc mon expérience est pas vrai indicateur pour savoir si l'un est supérieur.

Alors, les pensées?

C'est vraiment une question d'opinion, mais pour ce que ça vaut, je trouve trompeur de retourner l'article modifié si l'article est modifié en place. Par ailleurs, si predictPriceon va modifier l'élément, il doit avoir un nom qui indique qu'il va le faire (comme setPredictPriceou quelque).

Je préférerais (dans l'ordre)

1. predictPriceC'était une méthode deItem (modulo une bonne raison pour qu'il ne soit pas, ce qui pourrait bien y avoir dans votre conception globale) qui soit

   • Renvoyé le prix prévu, ou

   • Avait un nom comme à la setPredictedPriceplace

2. Cela predictPrice n'a pas changé Item, mais a renvoyé le prix prévu

3. Ce predictPricefut une voidméthode appeléesetPredictedPrice

4. Ce predictPriceretour this(pour le chainage des méthodes à d' autres méthodes sur tout cas , il est une partie de) (et a été appelé setPredictedPrice)

Au sein d' un paradigme de conception orientée objet, les choses ne doivent pas être en train de modifier des objets en dehors de l'objet lui - même. Toute modification de l'état d'un objet doit être effectuée via des méthodes sur l'objet.

Ainsi, en void predictPrice(Item item)tant que fonction membre d'une autre classe, c'est faux . Cela aurait pu être acceptable à l'époque de C, mais pour Java et C ++, les modifications d'un objet impliquent un couplage plus profond avec l'objet qui entraînerait probablement d'autres problèmes de conception en cours de route (lorsque vous refactorisez la classe et modifiez ses champs, maintenant que «PredictPrice» doit être remplacé par dans un autre fichier.

De retour en arrière d' un nouvel objet, ne dispose pas des effets secondaires associés, le paramètre est passé dans pas changé. Vous (la méthode PredictPrice) ne savez pas où ce paramètre est utilisé. La Itemclé d'un hachage est-elle quelque part? avez-vous changé son code de hachage en faisant cela? Est-ce que quelqu'un d'autre s'y accroche s'attend à ce qu'il ne change pas?

Ces problèmes de conception suggèrent fortement que vous ne devriez pas modifier des objets (je plaiderais pour l'immuabilité dans de nombreux cas), et si vous le faites, les modifications de l'état devraient être contenues et contrôlées par l'objet lui-même plutôt que quelque chose sinon en dehors de la classe.

Regardons ce qui se passe si l'on tripote les champs de quelque chose dans un hachage. Prenons un peu de code:

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

Et j'admets que ce n'est pas le plus grand code (accéder directement aux champs), mais il sert son objectif de démontrer un problème avec les données mutables utilisées comme clé d'un HashMap, ou dans ce cas, simplement mises dans un HashSet.

La sortie de ce code est:

true
false

Ce qui s'est passé, c'est que le hashCode utilisé lors de son insertion est l'endroit où l'objet se trouve dans le hachage. La modification des valeurs utilisées pour calculer le hashCode ne recalcule pas le hachage lui-même. C'est un danger de mettre n'importe quel objet mutable comme clé d'un hachage.

Ainsi, pour revenir à l'aspect original de la question, la méthode appelée ne "sait" pas comment l'objet qu'elle obtient en tant que paramètre est utilisé. Fournir la « permet de muter l'objet » comme la seule façon de faire cela signifie qu'il ya un certain nombre de bugs subtils qui peuvent se glisser. _{^{Comme perdre des valeurs dans un hachage ... sauf si vous ajoutez plus et le hachage gets rabâché - moi confiance , c'est un bug méchant à traquer (j'ai perdu la valeur jusqu'à ce que j'ajoute 20 autres éléments à la hashMap et puis soudain, il réapparaît).}}

• À moins qu'il n'y ait de très bonnes raisons de modifier l'objet, renvoyer un nouvel objet est la chose la plus sûre à faire.
• Quand il est une bonne raison de modifier l'objet, cette modification doit être effectuée par l'objet lui - même (invoquant des méthodes) plutôt que par une fonction externe qui peut tourner ses champs.
  • Cela permet de refaçonner l'objet avec un coût de maintenance moindre.
  • Cela permet à l'objet de s'assurer que les valeurs qui calculent son code de hachage ne changent pas (ou ne font pas partie de son calcul de code de hachage)

Connexe: Écraser et renvoyer la valeur de l'argument utilisé comme conditionnel d'une instruction if, à l'intérieur de la même instruction if

Je suis toujours la pratique de ne pas muter l'objet de quelqu'un d'autre. C'est-à-dire, uniquement des objets mutants appartenant à la classe / struct / whathaveyou dans laquelle vous vous trouvez.

Étant donné la classe de données suivante:

class Item {
    public double price = 0;
}

C'est acceptable:

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

Et cela est très clair:

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

Mais c'est beaucoup trop boueux et mystérieux à mes goûts:

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

La syntaxe est différente entre les différentes langues et il est inutile de montrer un morceau de code qui n'est pas spécifique à une langue car cela signifie des choses complètement différentes dans différentes langues.

En Java, Itemest un type de référence - c'est le type de pointeurs vers des objets qui sont des instances de Item. En C ++, ce type s'écrit Item *(la syntaxe pour la même chose est différente entre les langages). Donc Item predictPrice(Item item)en Java est équivalent à Item *predictPrice(Item *item)C ++. Dans les deux cas, c'est une fonction (ou méthode) qui prend un pointeur sur un objet et renvoie un pointeur sur un objet.

En C ++, Item(sans rien d'autre) est un type d'objet (en supposant que Itemc'est le nom d'une classe). Une valeur de ce type "est" un objet et Item predictPrice(Item item)déclare une fonction qui prend un objet et renvoie un objet. Puisque &n'est pas utilisé, il est transmis et renvoyé par valeur. Cela signifie que l'objet est copié lorsqu'il est transmis et copié lorsqu'il est renvoyé. Il n'y a pas d'équivalent en Java car Java n'a pas de types d'objets.

Alors c'est quoi? Beaucoup de choses que vous posez dans la question (par exemple, "je crois que cela fonctionne sur le même objet qui est passé ...") dépendent de la compréhension exacte de ce qui est passé et retourné ici.

La convention à laquelle j'ai vu adhérer les bases de code est de préférer un style clair dans la syntaxe. Si la fonction change de valeur, préférez passer par le pointeur

void predictPrice(Item * const item);

Ce style se traduit par:

• Appelez-le, vous voyez:

  PredictPrice (& my_item);

et vous savez qu'il sera modifié par votre respect du style.

• Sinon, préférez passer par const ref ou value lorsque vous ne voulez pas que la fonction modifie l'instance.

Bien que je n'aie pas une préférence marquée, je voterais que le retour de l'objet entraîne une meilleure flexibilité pour une API.

Notez qu'il n'a de sens que lorsque la méthode a la liberté de renvoyer un autre objet. Si votre javadoc le dit, @returns the same value of parameter ac'est inutile. Mais si votre javadoc le dit @return an instance that holds the same data than parameter a, le retour de la même instance ou d'une autre est un détail d'implémentation.

Par exemple, dans mon projet actuel (Java EE), j'ai une couche métier; pour stocker dans la base de données (et attribuer un ID automatique) un employé, disons, un employé j'ai quelque chose comme

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

Bien sûr, aucune différence avec cette implémentation car JPA renvoie le même objet après avoir attribué l'ID. Maintenant, si je suis passé à 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);
   ??????
 }

Maintenant à ????? J'ai trois options.

1. Définissez un setter pour Id, et je retournerai le même objet. Laid, car setIdsera disponible partout.

2. Faites un travail de réflexion intense et définissez l'identifiant dans l' Employeeobjet. Sale, mais au moins c'est limité au createEmployeerecord.

3. Fournissez un constructeur qui copie l'original Employeeet accepte également le idjeu.

4. Récupérez l'objet à partir de la base de données (peut-être nécessaire si certaines valeurs de champ sont calculées dans la base de données ou si vous souhaitez remplir une propriété).

Maintenant, pour 1. et 2. vous retournerez peut-être la même instance, mais pour 3. et 4. vous retournerez de nouvelles instances. Si d'après le principe que vous avez établi dans votre API que la "vraie" valeur sera celle retournée par la méthode, vous avez plus de liberté.

Le seul véritable argument contre lequel je peux penser est que, en utilisant les implémentations 1. ou 2., les personnes utilisant votre API peuvent ignorer l'attribution du résultat et leur code se briserait si vous passez aux implémentations 3. ou 4.).

Quoi qu'il en soit, comme vous pouvez le voir, ma préférence est basée sur d'autres préférences personnelles (comme interdire un setter pour les ID), alors peut-être que cela ne s'appliquera pas à tout le monde.

Lors du codage en Java, il faut essayer de faire correspondre l'utilisation de chaque objet de type mutable à l'un des deux modèles:

1. Exactement une entité est considérée comme le "propriétaire" de l'objet mutable et considère l'état de cet objet comme faisant partie du sien. D'autres entités peuvent détenir des références, mais devraient considérer la référence comme identifiant un objet qui appartient à quelqu'un d'autre.

2. Bien que l'objet soit mutable, personne qui en possède une référence n'est autorisé à le muter, rendant ainsi l'instance effectivement immuable (notez qu'en raison des bizarreries du modèle de mémoire de Java, il n'y a pas de bon moyen de faire en sorte qu'un objet effectivement immuable ait un thread- sécurité sémantique immuable sans ajouter un niveau supplémentaire d'indirection à chaque accès). Toute entité qui encapsule l'état à l'aide d'une référence à un tel objet et souhaite modifier l'état encapsulé doit créer un nouvel objet qui contient l'état approprié.

Je n'aime pas que les méthodes mutent l'objet sous-jacent et renvoient une référence, car elles donnent l'apparence de s'adapter au deuxième motif ci-dessus. Il y a quelques cas où l'approche peut être utile, mais le code qui va muter les objets devrait ressembler à ça; le code comme myThing = myThing.xyz(q);ressemble beaucoup moins à la mutation de l'objet identifié myThingque ne le ferait simplement myThing.xyz(q);.