Vermeiden Sie übermäßige Funktionsparameter: Klasse zentrierte oder Funktion zentrierten Ansatz?

Question

Wie würden Sie die folgenden schlechten Code zu beheben, die sich um zu viele Parameter passiert?

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);
}

Ich sehe zwei unterschiedliche Ansätze:

Ansatz 1: Setzen Sie alle Funktionen in einer Klasse

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
};

2 Ansatz: Nur Parameter als eine Klasse übergeben

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);
}

Was sind die Vor- und Nachteile der Ansätze?

Ein Vorteil des Ansatzes 1 ist, dass - wenn man die helper Funktionen virtuell machen - Sie können Unterklasse und überlasten sie und fügte hinzu, Flexibilität. Aber dann, in meinem Fall (außerhalb des Spielzeugmini Beispiel, dass ich gab) solche Helfer werden oft als Templat, so dass sie nicht virtuell sowieso sein kann.

Ein Vorteil des Ansatzes 2 ist, dass die Helferfunktionen können leicht von anderen Funktionen aufgerufen werden, auch.

( Diese Frage bezieht, aber nicht diskutieren diese beiden Alternativen.)

Answer 1

Kurze Antwort:

Happy New Year! Ich würde vermeiden Option # 1 und gehen nur mit der Option # 2, wenn die Parameter können in klare und logische Gruppen getrennt werden, dass sinnvoll weg von Ihrer Funktion.

Lange Antwort

Ich habe viele Beispiele von Funktionen gesehen, wie Sie von Mitarbeitern beschrieben. Ich werde mit Ihnen auf der Tatsache einig, dass es ein schlechter Code Geruch ist. Allerdings Parameter in eine Klasse Gruppierung nur damit Sie müssen nicht Parameter übergeben und zu entscheiden, eher willkürlich zu gruppieren sie auf diesen Hilfsfunktionen basieren, können führen zu mehr schlecht riecht. Sie müssen sich fragen, ob Sie die Lesbarkeit sind die Verbesserung und das Verständnis für andere, die nach Ihnen kommen.

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; }

Es können Sie Gruppen Dinge sein verständlich, weil es eine klare Unterscheidung zwischen Parameter ist. Dann würde ich vorschlagen, Ansatz # 2.

Auf der anderen Seite, Code, in dem ich oft übergeben worden bin wie folgt aussieht:

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; }

, die Blätter Sie mit einem Gefühl, dass diese Parameter sind nicht gerade freundlich zu Zerschlagung in etwas Sinnvolles klassenweise. Stattdessen würden Sie besser dran serviert Flipping Dinge um wie

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

und dann Aufruf

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

, die einen Schauer über den Rücken laufen können als diese kleine Stimme in deinem Kopf schreit „trocken, trocken, trocken!“ , Die man besser lesbar und somit wartbar ist?

Option # 1 ist ein No-Go in meinem Kopf, da es eine sehr gute Möglichkeit, jemand wird zu Satz einen der Parameter vergessen. Dies könnte sehr leicht zu einem harten führen Fehler zu erkennen, die einfachen Unit-Tests durchläuft. YMMV.

Answer 2

schrieb ich die Klassen und Strukturen, damit sie analysiert werden konnten wie folgt:

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 {}
}

Klassen verwenden, können Sie auf das Objekt handeln. Code, den das Punktobjekt verwendet wird eine Standard-Schnittstelle verwenden, um die Stelle zu bewegen. Wenn die Systemänderungen koordinieren und die Punktklasse geändert wird, um das Koordinatensystem zu erfassen, es in ist, wird dies die gleiche Schnittstelle ermöglichen, in mehreren Koordinatensysteme verwendet wird (nicht etwas zu zerbrechen). In diesem Beispiel ist es sinnvoll mit Ansatz zu gehen 1.

Wenn Ihr nur die Gruppierung verwandter Daten für Organisation Zwecke zugegriffen werden ...

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);
}

Dann macht es Sinn, mit Ansatz zu gehen 2.

Dies ist eine OO-Design Wahl, so gibt es mehr richtigen Lösungen und von dem, was wir gegeben sind, es ist schwierig, eine definitive Antwort zu geben.

Answer 3

Wenn P1, P2, etc ... sind wirklich nur Optionen (Flags, etc ...), und sind völlig unabhängig von einander, dann würde ich mit der zweiten Option gehen.

Wenn einige von ihnen neigen dazu, immer zusammen zu sein, dann vielleicht gibt es einige Klassen hier zu erscheinen warten, und ich würde mit der ersten Option gehen, aber wahrscheinlich nicht in einer großen Klasse (Sie haben wahrscheinlich tatsächlich mehr Klassen erstellen - - es ist schwer, ohne die Namen der aktuellen Parameter zu sagen).

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 # 2.

Wenn Sie jedoch schwer über die Domain denken, werden Sie wahrscheinlich feststellen, dass es eine sinnvolle Gruppierung des Parameterwertes ist, und sie können in Ihrer Domain zu einem gewissen Einheit zugeordnet werden.

Dann können Sie Instanzen dieses Objekt erstellen, übergeben Sie es sich in Ihrem Programm.

Normalerweise könnte dies so etwas wie ein Application-Objekt oder ein Objekt Session, etc.

Answer 5

Der Ansatz sie in eine Klasse setzen „Gefühl“ nicht korrekt. Es scheint wie ein Versuch, alten Code nachrüsten zu suchen, wie etwas, das es nicht ist. Im Laufe der Zeit könnte es möglich sein, zu einem „objektorientierten“ Stil des Code zu bewegen, aber es scheint, eher würde es sein eine schlechte Mischung von zwei Paradigmen enden. Aber das ist vor allem auf meinen Gedanken, was mit einigem nicht-OO-Code, dass ich die Arbeit mit passieren würde.

So zwischen diesen beiden Optionen, würde ich denke, dass die Struktur-Option ist ein wenig besser. Er verlässt die Flexibilität offen für eine andere Funktion, die Parameter in eine Struktur verpacken zu können und eine der Helferfunktionen aufrufen. Aber diese Methode mir scheint, wie es ein Problem mit dem Selbst Dokumentation Aspekte hat. Wenn es notwendig wird, einen Aufruf an helper1 von einem anderen Standort hinzuzufügen, ist es nicht so klar, welche Parameter müssen an sie übergeben werden.

So, während es wie eine Reihe von unten Stimmen fühlt sich meinen Weg kommen, glaube ich, der beste Weg, es zu nicht zu ändern ist. Die Tatsache, dass ein guter Editor Sie die Parameterliste zeigen wird, wenn Sie in einem neuen Aufruf zu einer der Funktionen Typ macht es ziemlich einfach es richtig zu machen. Und die Kosten, wenn es in einem hohen Verkehrsbereich ist alles nicht so toll. Und mit 64-Bit-Umgebungen ist es noch weniger, weil mehrere der Parameter (ist es 4?) Wird in der Regel in den Registern gesendet.

Hier ist eine weitere Frage im Gespräch darüber mit vielen Responder mit einem Gewicht von in.

Answer 6

Ich weiß es nicht. Es hängt wirklich davon ab, was die Parameter sind . Es gibt keine magische Entwurfsmuster „make 20 Funktionsparameter verschwinden“ Sie aufrufen können. Das Beste, was Sie tun können, ist Refactoring Code. Je nachdem, was die Parameter darstellen, könnte es sinnvoll Gruppe sein, einige von ihnen in eine Parameterklasse oder Put alles in ein monolithischen Klasse, oder teilen Sie die ganze Sache aus in mehrere Klassen. Eine Option, die arbeiten kann irgendeine Form von currying ist, vorbei die Objekte ein wenig in einer Zeit, jedes Mal ein neues Objekt, an dem der nächste Aufruf Nachgeben kann aufgerufen werden, um die nächste Partie der Parameter. Dies ist besonders sinnvoll, wenn einige Parameter oft das gleiche über Anrufe bleiben.

Eine einfachere Variante könnte sein, alles in einer Klasse wickeln, wo einige Parameter in dem Konstruktor übergeben werden (die, die typischerweise das gleiche für einen ganzen Stapel von Anrufen bleiben), und andere, die, die versorgt werden müssen per-Call, da sie ständig ändert, sind in dem eigentlichen Funktionsaufruf übergibt (wahrscheinlich ein überlasteter operator()) ist.

Ohne die tatsächliche zu wissen bedeutet des Codes, ist es unmöglich zu sagen, wie es am besten Refactoring werden.

Answer 7

Verwenden Sie Klassen nicht als reine Datenspeicherung und keine Felder verwenden, um Daten zu Methoden zu übergeben. Normalerweise, wenn ein Verfahren zu viele Parameter erfordert macht es zu viel, so sollten Sie die Funktionalität von ihm in getrennte Methoden extrahieren. Vielleicht könnten Sie ein konkretes Beispiel für Ihre Methoden bieten, so dass wir sie aus einer größeren Perspektive diskutieren.