Pergunta

Como você corrigir o seguinte código ruim que passa muitos parâmetros redor?

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

Eu vejo duas abordagens diferentes:

Abordagem 1:Colocar todas as funções em uma 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
};

Abordagem 2:Basta passar o parâmetro, como uma 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);
}

Quais são as vantagens e desvantagens das abordagens?

Uma vantagem da Abordagem 1 é que -- se você faz o helper funções virtuais -- você pode subclasse e sobrecarga, aumentando a flexibilidade.Mas então, no meu caso (fora do brinquedo mini exemplo que eu dei), tais auxiliares são muitas vezes de modelo, então eles não podem ser virtual de qualquer maneira.

Uma vantagem da Abordagem 2 é que as funções de ajuda podem facilmente ser chamado a partir de outras funções, também.

(Este a pergunta é relacionado, mas não explica essas duas alternativas.)

Foi útil?

Solução

Resposta Curta:

Feliz Ano Novo!Eu gostaria de evitar opção #1 e apenas ir com a opção #2 se os parâmetros podem ser separados em claro e lógico grupos que fazem sentido, longe de sua função.

Tempo De Resposta

Eu tenho visto muitos exemplos de funções como você descreveu, a partir de colegas de trabalho.Eu vou concordar com você sobre o fato de que é um mau código de cheiro.No entanto, o agrupamento de parâmetros em uma classe apenas para que você não precise passar parâmetros e decidir arbitrariamente, ao invés de agrupá-los com base nessas funções auxiliares podem levar a mais mau cheiro.Você tem que perguntar a si mesmo se você está melhorando a legibilidade e compreensão para com os outros que vêm depois de você.

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á você pode agrupar coisas para ser mais compreensível, uma vez que há uma clara distinção entre os parâmetros.Então eu gostaria de sugerir a abordagem #2.

Por outro lado, o código em que tenho sido entregue, muitas vezes, se parece com:

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

o que deixa você com uma sensação de que esses parâmetros não são exatamente amigável de quebrar-se em algo significativo classe-sábios.Em vez disso, você seria melhor fora servido virar as coisas ao redor, como

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

e, em seguida, chamando-o de

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

que pode enviar tremores sua coluna como aquela pequena voz dentro de sua cabeça grita "SECO, SECO, SECO!" Qual é o mais legível e, portanto, fácil de manter?

Opção #1 é não ir na minha cabeça, como há uma boa possibilidade de que alguém vai se esqueça de definir um dos parâmetros.Isso pode muito facilmente levar a um rígido para detectar o erro que passa de simples testes de unidade.YMMV.

Outras dicas

Eu reescreveu as classes e structs, portanto, pode ser analisada como se 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 {}
}

O uso de classes permite-lhe agir sobre o objeto.O código que usa o objeto de ponto irá usar uma interface padrão para mover o ponto de volta.Se o sistema de coordenadas alterações e a classe ponto é modificado para detectar o sistema de coordenadas é, isso vai permitir que a mesma interface para ser utilizado em vários sistemas de coordenadas (não quebrar nada).Neste exemplo, faz sentido para ir com a abordagem 1.

Se o seu apenas o agrupamento de dados relacionados que podem ser acessados para fins de organização...

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

Em seguida, faz sentido ir com a Abordagem 2.

Este é um design OO escolha, há várias soluções corretas e de que estamos dada é difícil dar uma resposta definitiva.

Se P1, P2, etc ... são realmente apenas opções (sinalizadores, etc ...) e não estiverem completamente relacionados um com o outro, então eu iria com a segunda opção.

Se alguns deles tendem a estar sempre juntos, talvez haja algumas aulas esperando para aparecer aqui, e eu iria com a primeira opção, mas provavelmente não em uma grande classe (você provavelmente tem várias aulas para criar - é difícil dizer sem os nomes dos parâmetros reais).

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

Opção 2.

No entanto, se você pensar muito no domínio, provavelmente descobrirá que existe um agrupamento significativo dos valores dos parâmetros e eles podem ser mapeados para alguma entidade em seu domínio.

Em seguida, você pode criar instâncias desse objeto, passá -lo para o seu programa.

Normalmente, isso pode algo como um objeto de aplicativo, ou um objeto de sessão, etc.

A abordagem de colocá -los em uma classe não "parece" correta. Parece uma tentativa de adaptar o código antigo para parecer algo que não é. Com o tempo, pode ser possível avançar em direção a um estilo de código "orientado a objetos", mas parece mais provável que acabasse sendo uma mistura ruim de dois paradigmas. Mas isso se baseia principalmente em meus pensamentos sobre o que aconteceria com algum código que não é o OO com o qual trabalho.

Então, entre essas duas opções, eu acho que a opção da estrutura é um pouco melhor. Ele deixa a flexibilidade aberta para que outra função seja capaz de empacotar os parâmetros em uma estrutura e chamar uma das funções auxiliares. Mas esse método para mim parece ter um problema com o aspecto de auto-documentação. Se for necessário adicionar uma chamada ao Helper1 a partir de algum outro local, não é tão claro quais parâmetros devem ser transmitidos a ele.

Então, embora pareça que vários votos de queda vindo em minha direção, acho que a melhor maneira é não mudar isso. O fato de um bom editor mostrar a lista de parâmetros quando você digitar uma nova chamada para uma das funções torna bastante simples acertar. E o custo, a menos que esteja em uma área de tráfego intenso, não é tão grande assim. E com ambientes de 64 bits, é ainda menor porque mais dos parâmetros (são 4?) Normalmente são enviados nos registros.

Aqui é uma pergunta relacionada falando sobre isso com muitos respondentes pesando.

Eu não sei.Ela realmente depende de que os parâmetros são.Não há mágica "fazer 20 parâmetros de função desaparecer" padrão de design que você pode chamar.O melhor que você pode fazer é refatorar seu código.Dependendo de quais os parâmetros representam, pode ser significativo para o grupo, alguns deles em um parâmetro de classe, ou colocar tudo em um monolítico de classe, ou dividir a coisa toda em várias classes.Uma opção que pode funcionar é alguma forma de pelaria, passando a objetos um pouco de cada vez, a cada vez, produzindo um novo objeto na qual a próxima chamada pode ser invocada, passando o próximo lote de parâmetros.Este especial faz sentido se alguns parâmetros, muitas vezes, permanecer o mesmo através de chamadas.

Uma variante mais simples do que este poderia ser para juntar tudo em uma classe, onde alguns parâmetros são passados no construtor (que normalmente permanecem as mesmas em todo um lote de chamadas), e os outros, aqueles que devem ser fornecidos por chamada como eles estão constantemente mudando, são passados na real chamada de função (provavelmente um sobrecarregado operator()).

Sem saber o real significado de seu código, é impossível dizer como ele pode ser melhor refatorado.

Não use as classes como armazenamento de dados puro e não use campos para passar dados para métodos. Geralmente, se um método requer muitos parâmetros, ele faz muito, você deve extrair a funcionalidade para métodos separados. Talvez você possa fornecer um exemplo mais concreto de seus métodos para que possamos discutir isso de uma perspectiva maior.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top