Evitare parametri di funzione eccessivi:Approccio centrato sulla classe o centrato sulla funzione?

Question

Come risolveresti il ​​seguente codice errato che trasmette troppi parametri?

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

Vedo due approcci diversi:

Approccio 1:Metti tutte le funzioni in una 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
};

Approccio 2:Basta passare i parametri come 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);
}

Quali sono i vantaggi e gli svantaggi degli approcci?

Un vantaggio dell'Approccio 1 è che: se realizzi il helper funzioni virtuali: puoi creare sottoclassi e sovraccaricarle, aggiungendo flessibilità.Ma poi, nel mio caso (a parte il mini esempio del giocattolo che ho fornito) tali aiutanti sono spesso modellati, quindi non possono comunque essere virtuali.

Un vantaggio dell'approccio 2 è che le funzioni di supporto possono essere facilmente richiamate anche da altre funzioni.

(Questo la domanda è correlata, ma non discute queste due alternative.)

Answer 1

Risposta breve:

Buon Anno!Eviterei l'opzione n. 1 e opterei per l'opzione n. 2 solo se i parametri possono essere separati in gruppi chiari e logici che abbiano senso lontano dalla tua funzione.

Risposta lunga

Ho visto molti esempi di funzioni come hai descritto dai colleghi.Sono d'accordo con te sul fatto che si tratta di un cattivo odore di codice.Tuttavia, raggruppare i parametri in una classe solo per non doverli passare e decidere in modo piuttosto arbitrario di raggrupparli in base a quelle funzioni di supporto può portare a più cattivi odori.Devi chiederti se stai migliorando la leggibilità e la comprensione per gli altri che verranno dopo di te.

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

Lì puoi raggruppare le cose per essere più comprensibili perché c'è una chiara distinzione tra i parametri.Quindi suggerirei l'approccio n. 2.

D'altra parte, il codice che mi è stato consegnato spesso assomiglia a:

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

il che ti lascia con la sensazione che questi parametri non siano esattamente adatti a scomporsi in qualcosa di significativo dal punto di vista della classe.Invece faresti meglio a girare cose come

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

e poi chiamarlo

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

Che può inviare i brividi sulla schiena mentre quella piccola voce all'interno della testa urla "asciutta, asciutta, asciutta!" Quale è più leggibile e quindi mantenebile?

L'opzione n. 1 è da evitare nella mia testa poiché esiste un'ottima possibilità che qualcuno si dimentichi di impostare uno dei parametri.Ciò potrebbe facilmente portare a un bug difficile da rilevare che supera semplici test unitari.YMMV.

Answer 2

ho riscritto le classi e le strutture in modo da poter essere analizzati come segue:

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

Utilizzo di classi consente di agire sul vostro oggetto. Il codice che utilizza l'oggetto punto utilizzerà un'interfaccia standard per spostare il punto intorno. Se la coordinata modifiche al sistema e la classe point viene modificato per rilevare il sistema di coordinate è in, ciò consentirà la stessa interfaccia da utilizzare in sistemi multipli (non rompere nulla) coordinate. In questo esempio, ha senso andare con approccio 1.

Se i dati relativi solo raggruppamento a cui accedere per scopi organizzativi ...

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

Quindi ha senso andare con Approccio 2.

Questa è una scelta di progettazione OO quindi non c'è più soluzioni corrette e da quello che c'è dato è difficile dare una risposta definitiva.

Answer 3

Se p1, p2, ecc ... sono in realtà solo le opzioni (bandiere, ecc ...), e sono completamente slegati tra loro, quindi vorrei andare con la seconda opzione.

Se alcuni di loro tendono ad essere sempre insieme, allora forse ci sono alcune classi in attesa di comparire qui, e vorrei andare con la prima opzione, ma probabilmente non in un unico grande classe (probabilmente effettivamente avere diverse classi per creare - - è difficile dire senza i nomi dei parametri attuali).

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

Opzione # 2.

Tuttavia, se si pensa duro circa il dominio, probabilmente troverete che c'è un raggruppamento significativa dei valori dei parametri e può essere mappato a qualche entità nel dominio.

Quindi, è possibile creare istanze di questo oggetto, passarlo al tuo programma.

In genere questo potrebbe qualcosa di simile a un oggetto Application, o di un oggetto Session, ecc.

Answer 5

L'approccio di metterli in una classe non "sentire" corretta. Sembra un tentativo di retrofit vecchio codice in cerca come qualcosa che non lo è. Nel corso del tempo, potrebbe essere possibile a muoversi verso uno stile "object-oriented" del codice, ma sembra più probabile che finirebbe per essere un cattivo mix di due paradigmi. Ma che si basa principalmente sui miei pensieri su ciò che sarebbe accaduto con un po 'di codice non-OO che io lavoro con.

Così tra queste due opzioni, penserei l'opzione struct è un po 'meglio. Si lascia la flessibilità aperto per un'altra funzione per essere in grado di confezionare i parametri in una struttura e chiamare una delle funzioni di supporto. Ma questo metodo mi sembra che ha un problema con l'aspetto di auto-documentazione. Se diventa necessario aggiungere una chiamata a Helper1 da qualche altra posizione, non è chiaro quali parametri devono essere passati ad esso.

Così, mentre ci si sente come un numero di voti verso il basso a venire il mio modo, penso che il modo migliore è quello di non cambiarlo. Il fatto che un buon editor vi mostrerà la lista dei parametri quando si digita una nuova chiamata a una delle funzioni lo rende piuttosto semplice da farlo bene. E il costo a meno che non si trova in una zona ad alto traffico non è poi così grande. E con ambienti a 64 bit è ancora meno, perché più dei parametri (è 4?) Sono tipicamente inviati in registri.

Qui è una questione connessa a parlare di questo con un sacco di responder pesatura a.

Answer 6

Non lo so. In realtà dipende quali sono i parametri . Non c'è nessun modello di progettazione magica "fanno 20 parametri di funzione scompaiono" è possibile richiamare. Il meglio che puoi fare è refactoring del codice. A seconda di quali sono i parametri rappresentano, potrebbe essere significativo per gruppo alcuni di loro in una classe di parametri, o mettere tutto in una sola classe monolitica, o dividere il tutto fuori in più classi. Una possibilità che può funzionare sia qualche forma di currying, passando gli oggetti un po 'alla volta, ogni volta ottenendo un nuovo oggetto su cui la chiamata successiva può essere invocato, passando il successivo gruppo di parametri. Questo rende particolarmente senso se alcuni parametri spesso rimangono le stesse chiamate in tutto.

Una variante più semplice di questo potrebbe essere per avvolgere tutto in una classe, dove sono passati alcuni parametri nel costruttore (quelli che in genere rimangono le stesse per tutta una serie di chiamate), e altri, quelli che devono essere forniti per chiamata come sono in continua evoluzione, sono passati nella chiamata di funzione effettiva (probabilmente un operator() sovraccarico).

Senza conoscere la reale significa del codice, è impossibile dire come può essere meglio refactoring.

Answer 7

Non utilizzare classi come la memorizzazione dei dati puri e non utilizzare i campi per passare i dati ai metodi. Di solito se un metodo richiede troppi parametri che fa troppo, così si dovrebbe estrarre la funzionalità da esso in metodi separati. Forse si potrebbe fornire un esempio più concreto dei tuoi metodi in modo che possiamo discutere da una prospettiva più grande.