Évitez les paramètres de la fonction excessive: approche centrée sur la classe ou centrée fonction?

Question

Comment voulez-vous fixer le mauvais code suivant qui passe trop de paramètres autour?

void helper1(int p1, int p3, int p5, int p7, int p9, int p10) {
  // ...
}

void helper2(int p1, int p2, int p3, int p5, int p6, int p7, int p9, int p10) {
  // ...
}

void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8, 
         int p9, int p10) {
  helper1(p1, p3, p5, p7, p9, p10);
  helper2(p1, p2, p3, p5, p6, p7, p9, p10);
}

Je vois deux approches différentes:

Approche 1: Mettez toutes les fonctions dans une classe

class Foo {
  private:
    int p1, p2, p3, p4, p5, p6, p7, p8, p9, p10;
    void helper1() {}
    void helper2() {}
  public:
    void foo() {
      helper1();
      helper2();
    }
    // add constructor
};

Approche 2: Il suffit de passer des paramètres en tant que classe

struct FooOptions {
    int p1, p2, p3, p4, p5, p6, p7, p8, p9, p10;
};

void helper1(const FooOptions& opt) {
  // ...
}

void helper2(const FooOptions& opt) {
  // ...
}

void foo(const FooOptions& opt) {
  helper1(opt);
  helper2(opt);
}

Quels sont les avantages et les inconvénients des approches?

Un avantage de l'approche 1 est que - si vous faites les fonctions de helper virtuel - vous pouvez sous-classe et les surcharger, en ajoutant la flexibilité. Mais, dans mon cas (en dehors du jouet mini-exemple que j'ai donné) ces aides sont souvent basés sur des modèles, de sorte qu'ils ne peuvent pas être virtuel de toute façon.

Un avantage de l'approche 2 est que les fonctions d'aide peuvent facilement être appelées à partir d'autres fonctions aussi.

( Cette question est liée, mais ne discute pas ces deux alternatives.)

Answer 1

Réponse courte:

Bonne année! Il faut éviter l'option 1 et seulement aller avec l'option # 2 si les paramètres peuvent être séparés en groupes clairs et logiques qui ont du sens loin de votre fonction.

Réponse longue

Je l'ai vu de nombreux exemples de fonctions que vous avez décrit des collègues. Je suis d'accord avec vous sur le fait que ce soit une mauvaise odeur de code. Toutefois, le regroupement des paramètres dans une classe juste pour que vous ne devez pas passer des paramètres et décider de façon plutôt arbitraire de les regrouper en fonction de ces fonctions d'aide peut conduire à des odeurs plus mauvaises. Vous devez vous demander si vous améliorez la lisibilité et la compréhension pour les autres qui viennent après vous.

calcTime(int p1, int p2, int p3, int p4, int p5, int p6) {
    dist = calcDistance( p1, p2, p3 );
    speed = calcSpeed( p4, p5, p6 );
    return speed == 0 : 0 ? dist/speed; }

vous pouvez les choses de groupe pour être plus compréhensible parce qu'il ya une distinction claire entre les paramètres. Ensuite, je suggère l'approche # 2.

D'autre part, le code dans lequel j'ai remis ressemble souvent à:

calcTime(int p1, int p2, int p3, int p4, int p5, int p6) {
    height = height( p1, p2, p3, p6 );
    type = getType( p1, p4, p5, p6 );
    if( type == 4 ) {
        return 2.345; //some magic number
    }
    value = calcValue( p2, p3, type ); //what a nice variable name...
    a = resetA( p3, height, value );
    return a * value; }

qui vous laisse avec un sentiment que ces paramètres ne sont pas exactement amicale de briser en quelque chose de significatif classe sage. Au lieu de cela, vous seriez mieux servi en feuilletant les choses autour comme

calcTime(Type type, int height, int value, int p2, int p3)

puis appeler

calcTime(
    getType( p1, p4, p5, p6 ),
    height( p1, p2, p3, p6 ),
    p3,
    p4 );

qui peut envoyer des frissons dans le dos comme cette petite voix dans votre tête crie « sec, sec, sec! » Lequel est plus lisible et donc maintenable?

Option n ° 1 est un non-droit dans ma tête comme il y a une personne très bonne possibilité va oublier de mettre un des paramètres. Cela pourrait très facilement conduire à un bug difficile à détecter qui passe des tests simples unitaires. YMMV.

Answer 2

Je réécris les classes et structs afin qu'ils puissent être analysés comme suit:

class Point {
      private:
        int x, y
        void moveUp() {}
        void moveRight() {}
      public:
        void moveUpAndRight() {
          moveUp();
          moveRight();
        }
        // add constructor
    };

struct Point {
  int x, y;
};

void moveUp {}
void moveRight {}
void moveUpAndRight {
  moveUp {}
  moveRight {}
}

Utilisation des classes vous permet d'agir sur votre objet. Le code qui utilise l'objet point utilisera une interface standard pour déplacer le point autour. Si les coordonnées des changements du système et la classe de points est modifié pour détecter le système de coordonnées est en, cela permettra à la même interface à utiliser dans plusieurs systèmes de coordonnées (rien casser). Dans cet exemple, il est logique d'aller à l'approche 1.

Si votre juste regroupement des données associées à accéder à des fins d'organisation ...

struct Dwarves {
  int Doc, Grumpy, Happy, Sleepy, Bashful, Sneezy, Dopey;
}

void killDwarves(Dwarves& dwarves) {}
void buryDwarves(Dwarves& dwarves) {}

void disposeOfDwarves(Dwarves& dwarves) {
  killDwarves(dwarves);
  buryDwarves(dwarves);
}

Alors il est logique d'aller avec l'approche 2.

Ceci est un choix de conception OO donc il y a plusieurs solutions correctes et de ce que nous sommes donné, il est difficile de donner une réponse définitive.

Answer 3

Si p1, p2, etc ... sont vraiment seulement des options (drapeaux, etc ...), et sont complètement indépendants les uns des autres, alors je vais avec la deuxième option.

Si certains d'entre eux ont tendance à être toujours ensemble, alors peut-être il y a des cours en attente d'apparaître ici, et j'aller avec la première option, mais probablement pas dans une grande classe (vous avez probablement fait plusieurs classes pour créer - - il est difficile de dire sans les noms des paramètres réels).

class Something {
   int p1, p3, p5 ...;
   void helper1a();
};

class SomethingElse {
   int p7, p9, p10 ...;
   void helper1b();
};

class Options {
   int p2, p4, ...
};

void foo(Something &s, SomethingElse & else, Options &options) {
  helper2(options);
  s.helper1a();
  else.helper1b();
}

Answer 4

Option n ° 2.

Cependant, si vous pensez dur sur le domaine, vous constaterez probablement qu'il ya un regroupement significatif des valeurs de paramètres et ils peuvent être mis en correspondance avec une entité dans votre domaine.

Ensuite, vous pouvez créer des instances de cet objet, le transmettre à votre programme.

Typiquement, cela pourrait quelque chose comme un objet d'application, ou un objet Session, etc.

Answer 5

L'approche de les mettre dans une classe ne pas « sentir » correcte. Il semble comme une tentative de moderniser l'ancien code en ressemblant à quelque chose qu'il n'est pas. Au fil du temps, il pourrait être possible de se déplacer vers un style de code « orienté objet », mais il semble plus probable qu'il finirait par être un mauvais mélange de deux paradigmes. Mais ce qui est principalement basé sur mes pensées de ce qui se passerait avec un code non-OO que je travaille avec.

Alors entre ces deux options, je pense l'option struct est un peu mieux. Il laisse la flexibilité ouverte pour une autre fonction pour pouvoir emballer les paramètres dans une structure et appeler l'une des fonctions d'aide. Mais cette méthode me semble que cela a un problème avec l'aspect auto-documenté. S'il devient nécessaire d'ajouter un appel à Helper1 d'un autre endroit, il est aussi clair que les paramètres doivent être transmis.

Ainsi, alors qu'il se sent comme un nombre de voix vers le bas à venir mon chemin, je pense que la meilleure façon est de changer pas. Le fait qu'un bon éditeur vous montre la liste des paramètres lorsque vous tapez dans un nouvel appel à l'une des fonctions, il est assez simple de faire les choses. Et le coût moins que ce soit dans une zone de forte circulation est pas terrible. Et avec les environnements 64 bits, il est encore moins parce que plus des paramètres (est-il 4?) Sont généralement envoyés dans des registres.

est une question connexe à parler de cela avec beaucoup d'intervenants poids dans.

Answer 6

Je ne sais pas. Cela dépend vraiment de ce que les paramètres sont . Il n'y a pas modèle de conception magique « faire disparaître 20 paramètres de la fonction », vous pouvez invoquer. Le mieux que vous pouvez faire est refactoriser votre code. En fonction de ce que les paramètres représentent, il pourrait être utile de regrouper certains d'entre eux dans une classe de paramètres, ou tout mettre dans une classe monolithique, ou diviser le tout en en plusieurs classes. Une option qui peut fonctionner est une certaine forme de corroyage, en passant les objets un peu à la fois, à chaque fois ce qui donne un nouvel objet sur lequel peut être invoqué le prochain appel, en passant le lot suivant des paramètres. Cela rend particulièrement de sens que si certains paramètres restent souvent les mêmes appels à travers.

Une variante plus simple de cela pourrait être pour envelopper le tout dans une classe, où certains paramètres sont passés dans le constructeur (ceux qui restent généralement les mêmes dans un lot d'appels), et d'autres, ceux qui doivent être fournis par appel comme ils sont en constante évolution, sont passés dans l'appel de fonction réelle (probablement une surcharge de operator()).

Sans connaître la réelle signifie de votre code, il est impossible de dire comment il peut être mieux refactorisé.

Answer 7

Ne pas utiliser des classes que le stockage de données pures et ne pas utiliser les champs pour transmettre des données à des méthodes. En général, si une méthode nécessite trop de paramètres, il en fait trop, vous devriez donc extraire la fonctionnalité de celui-ci dans des méthodes distinctes. Peut-être que vous pourriez donner un exemple plus concret de vos méthodes afin que nous puissions en discuter d'une perspective plus large.