過剰な関数パラメータを避けてください。クラス中心のアプローチか、機能中心のアプローチか?
https://stackoverflow.com/questions/1988914
-
22-09-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
Russian質問
あまりにも多くのパラメーターを渡している次の悪いコードを修正するにはどうすればよいでしょうか?
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);
}
私は 2 つの異なるアプローチを考えています。
アプローチ 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 の利点は、ヘルパー関数を他の関数からも簡単に呼び出すことができることです。
(これ 質問は関連していますが、これら 2 つの選択肢については説明しません。)
解決
短い答え：
ハッピーニューイヤー！私はオプション＃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 );
「DRY、DRY、DRYを！」どちらがより読みやすく、したがって、保守性とは？
誰かがパラメータのセット1に忘れてしまう非常に良い可能性があるとして、オプション＃1は私の頭の中で何-行くではありません。これは非常に簡単なユニットテストに合格するバグを検出することは困難につながる可能性があります。 YMMVます。
他のヒント
私は、クラスや構造体を書き直します：
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と行くことは理にかなっています。
これはオブジェクト指向設計の選択は非常に複数の正解があると我々が与えられているものから、それは決定的な答えを与えることは難しいです。
、その後、私は2番目の選択肢となるだろう。
それらのいくつかはいつも一緒になりがち場合は、は、多分ここに表示されるように待機しているいくつかのクラスがあり、私は最初のオプションで行くだろうが、おそらく一つの大きなクラスで（おそらく実際に作成するには、いくつかのクラスを持っていません - - それは、実際のパラメータの名前）なしで言うのは難しいです。
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();
}
オプション＃2。
ただし、ドメインに関するハード思うなら、あなたはおそらく、パラメータ値の意味のあるグループがあることがわかりますし、彼らはあなたのドメイン内のいくつかのエンティティにマッピングすることができます。
次に、あなたのプログラムにそれを渡し、このオブジェクトのインスタンスを作成することができます。
一般的に、このかもしれないApplicationオブジェクトのようなもの、またはSessionオブジェクト、など。
クラスにそれらを置くのアプローチが正しい「感触」はありません。それがないことを何かのように見えるに古いコードを改造しようとする試みのように思えます。時間が経つにつれて、コードの「オブジェクト指向」スタイルに向かって移動することは可能かもしれないが、可能性が高く、それは2つのパラダイムの悪いミックスになってしまうだろうと思われます。しかし、それは主に私の仕事を持つことをいくつかの非オブジェクト指向のコードで何が起こるかの私の考えに基づいています。
だから、私は最善の方法は、それを変更しないことだと思います。あなたは機能の一つに、新しいコールに入力したときに良いエディタはあなたにパラメータリストが表示されますという事実は、右のそれを得るために、それはかなり簡単になります。そして、それは、高トラフィックエリア内にある場合を除き、コストはすべてが素晴らしいではありません。そして、それはさらに少ないため、複数のパラメータのある64ビットの環境で（それは？4である）は、典型的には、レジスタに送信されます。
ここには、計量応答者の多くと、このことについて話して、関連する質問ですインチ
私は知りません。それは本当にパラメータは のあるものに依存します。あなたが呼び出すことができるデザインパターン魔法の「20個の関数のパラメータが消えるように」ありません。あなたができる最善のは、あなたのコードのリファクタリングです。パラメータが何を表しているかに応じて、1つのモノリシッククラスにそれらのいくつかのパラメータのクラスにグループに意味のある、またはプットすべてであるかもしれない、または複数のクラスの中に全部を分割します。月作業は、一度に少しオブジェクト渡し、カリー化のいくつかの形態であることが一つの選択肢は、新しいオブジェクトが得られるたびにその上に次のコールは、パラメータの次のバッチを渡し、起動することができます。これは特に、いくつかのパラメータは、多くの場合、通話全体で同じままであれば理にかなっています。
このシンプルな変種は、いくつかのパラメータは、コンストラクタに（通常のコールのバッチ全体で同じにとどまるもの）、および他の人を渡されたクラスで供給されなければならないものをすべてをラップすることができコールごとの彼らは常に変化しているとして、実際の関数呼び出し（おそらく過負荷にoperator()）に渡されます。
のコードの、それはそれは最高のリファクタリングすることができますどのように言うことは不可能です。
を意味し、実際のを知らずにDoが純粋なデータストレージとしてクラスを使用しないとメソッドにデータを渡すためにフィールドを使用しないでください。この方法は、あまりにも多くのパラメータを必要と通常ならば、それはあまりにも、あなたが別の方法にそれから機能を抽出しなければならないん。私たちはもっと大きな視点から、それを議論することができますので、たぶん、あなたはあなたの方法のより具体的な例を提供することができます。
