6

您将如何修复以下传递过多参数的错误代码？

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

我看到两种不同的方法：

方法1：将所有函数放在一个类中

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：只需将参数作为类传递

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

这些方法的优点和缺点是什么？

方法 1 的一个优点是——如果你将helper函数设为虚拟——你可以继承和重载它们，从而增加灵活性。但是，在我的情况下（在我给出的玩具迷你示例之外），这些助手通常是模板化的，所以它们无论如何都不能是虚拟的。

方法 2 的一个优点是辅助函数也可以很容易地从其他函数中调用。

（这个问题是相关的，但没有讨论这两种选择。）

4

7 回答 7

7

简短的回答：

新年快乐！我会避免使用选项#1，并且只使用选项#2，如果参数可以分成清晰且逻辑的组，这些组对你的功能有意义。

长答案

正如您从同事那里描述的那样，我已经看到了许多功能示例。我同意你的观点，即这是一种糟糕的代码气味。但是，将参数分组到一个类中只是为了您不必传递参数并根据这些辅助函数随意决定对它们进行分组可能会导致更难闻的气味。您必须问问自己，您是否正在提高其他人的可读性和理解力。

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

在那里，您可以将事物分组以更易于理解，因为参数之间有明显的区别。然后我会建议方法＃2。

另一方面，我收到的代码通常如下所示：

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

这让您感觉这些参数对于分解成有意义的类并不完全友好。相反，你最好把事情翻过来，比如

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

然后调用它

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

这可能会让你的脊椎发抖，因为你脑袋里的那个小声音尖叫着“干，干，干！” 哪一个更具可读性和可维护性？

选项 #1 在我的脑海中是不可行的，因为很有可能有人会忘记设置其中一个参数。这很容易导致难以检测到通过简单单元测试的错误。YMMV。

于 2010-01-01T17:12:17.977 回答
2

我重写了类和结构，因此可以对它们进行如下分析： 

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

使用类允许你对你的对象采取行动。使用点对象的代码将使用标准接口来移动点。如果坐标系发生变化并且点类被修改以检测它所在的坐标系，这将允许在多个坐标系中使用相同的接口（不会破坏任何东西）。在此示例中，使用方法 1 是有意义的。

如果您只是出于组织目的对要访问的相关数据进行分组...

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

那么使用方法 2 是有意义的。

这是一个面向对象的设计选择，因此有多种正确的解决方案，而且从我们所得到的情况来看，很难给出明确的答案。

于 2010-01-01T16:30:43.660 回答
1

如果 p1,p2,etc... 真的只是选项（标志等...），并且彼此完全无关，那么我会选择第二个选项。

如果他们中的一些人往往总是在一起，那么也许有一些类等待出现在这里，我会选择第一个选项，但可能不在一个大类中（你实际上可能有几个类要创建——这很难说没有实际参数的名称）。 

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();
}
于 2010-01-01T16:03:49.593 回答
1

将它们放入一个类的方法并不“感觉”正确。这似乎是一种尝试将旧代码改造成看起来不是的东西。随着时间的推移，可能会转向“面向对象”的代码风格，但它似乎更有可能最终成为两种范式的糟糕混合。但这主要是基于我对我使用的一些非 OO 代码会发生什么的想法。

所以在这两个选项之间，我认为 struct 选项要好一些。它为另一个函数留下了灵活性，以便能够将参数打包到一个结构中并调用其中一个辅助函数。但在我看来，这种方法在自我记录方面存在问题。如果需要从其他位置添加对 Helper1 的调用，则不清楚必须将哪些参数传递给它。

因此，虽然我感觉有很多反对票，但我认为最好的方法是不要改变它。当您键入对其中一个函数的新调用时，一个好的编辑器会向您显示参数列表，这一事实使得正确处理它变得非常简单。除非它在人流量大的区域，否则成本并不是那么高。而对于 64 位环境，它甚至更少，因为更多的参数（是 4 个？）通常在寄存器中发送。

这是一个相关的问题，有很多响应者参与讨论这个问题。

于 2010-01-01T16:30:24.067 回答
1

选项 #2。

但是，如果您仔细考虑域，您可能会发现参数值有一个有意义的分组，它们可以映射到您域中的某个实体。

然后，您可以创建此对象的实例，将其传递给您的程序。

通常，这可能类似于 Application 对象或 Session 对象等。

于 2010-01-01T17:21:54.880 回答
1

我不知道。这真的取决于参数是什么。没有可以调用的神奇的“使 20 个函数参数消失”设计模式。你能做的最好的就是重构你的代码。根据参数所代表的含义，将其中一些参数分组到一个参数类中，或者将所有内容放入一个整体类中，或者将整个事物分成多个类，这可能是有意义的。一个可行的选择是某种形式的柯里化，一次传递一点对象，每次产生一个新对象，可以在其上调用下一个调用，传递下一批参数。如果某些参数在调用中经常保持不变，这尤其有意义。

一个更简单的变体可能是将所有内容包装在一个类中，其中一些参数在构造函数中传递（通常在整批调用中保持相同的参数），而其他参数则必须在每次调用时提供由于它们不断变化，因此在实际的函数调用中传递（可能是重载的operator()）。

如果不知道代码的实际含义，就不可能说出如何最好地重构它。

于 2010-01-01T18:31:14.133 回答
1

不要将类用作纯数据存储，也不要使用字段将数据传递给方法。通常，如果一个方法需要太多参数，它就做的太多了，所以你应该把它的功能提取到单独的方法中。也许您可以提供一个更具体的方法示例，以便我们从更大的角度进行讨论。

于 2010-01-01T19:09:15.570 回答