booleanパラメータで動作を決定するのは間違っているのでしょうか？

私がこのパターンを使うのをやめたのは、ずいぶん前のことですが、理由は非常に単純で、メンテナンスコストです。 frobnicate(何か, forwards_flag)という関数がコード内で何度も呼び出されていることに気づき、forwards_flagの値としてfalse`が渡されたコード内のすべての場所を見つける必要がありました。 それを簡単に検索することはできないので、これはメンテナンスの頭痛の種になります。 また、それらのサイトごとにバグフィックスを行う必要がある場合、一つを見逃すと不幸な問題が発生する可能性があります。

しかし、この特定の問題は、アプローチを根本的に変えることなく、簡単に修正することができます：

enum FrobnicationDirection {
  FrobnicateForwards,
  FrobnicateBackwards;
};

void frobnicate(Object what, FrobnicationDirection direction);

このコードでは、FrobnicateBackwardsのインスタンスを検索するだけでよいことになります。 このコードを変数に代入して、いくつかの制御の糸をたどる必要があるコードがある可能性もありますが、実際には、この代替案が問題なく機能するほどまれなことだと感じています。

しかし、少なくとも原理的には、この方法でフラグを渡すことには別の問題があります。 それは、この設計を採用した一部の（一部の）システムでは、（フラグを使用する）コードの深くネストした部分の実装の詳細に関する知識を、（フラグにどの値を渡すかを知る必要がある）外層に公開し過ぎてしまう可能性があるということです。 Larry Constantineの用語を使うなら、この設計は、設定者とブーリアンフラグの使用者の間に、強すぎる結合があるのかもしれません。 Franky この質問に関しては、コードベースについて詳しく知らない限り、確実なことを言うのは難しいのですが。

あなたが挙げた具体的な例を挙げると、私はそれぞれにある程度の懸念を抱いていますが、それは主にリスクと正しさの理由からです。 つまり、システムがどのような状態にあるのかを示すフラグを渡す必要がある場合、それを考慮すべきコードがあるにもかかわらず、（この関数に渡されなかったために）パラメータをチェックしないことに気づくかもしれません。 つまり、誰かがパラメータを渡し忘れたために、バグが発生したのです。

また、ほとんどすべての関数に渡す必要があるシステム状態インジケータは、実際には本質的にグローバル変数であることを認める価値があります。 グローバル変数の欠点の多くが適用されます。 多くの場合、システム状態（あるいはユーザーの認証情報、システムのアイデンティティ）に関する知識を、そのデータに基づいて正しく動作する責任を持つオブジェクトにカプセル化する方が、より良い実践方法だと思います。 そして、生のデータとは別に、そのオブジェクトへの参照を渡します。 ここで重要なのは、href="http://en.wikipedia.org/wiki/Encapsulation_(object-oriented_programming)">カプセル化という概念です。

はい、これはおそらくコードのにおいであり、理解するのが難しく、簡単に再利用される可能性が低い、変更不可能なコードにつながります。

他のポスターが指摘しているように、コンテキストがすべてです。 (ドン&#39。;it&#39なら強引に行きます。;s 1回限りの場合、または慣行が故意に発生した技術的負債として後で再係合されることが認められている場合。) ただし、大まかに言えば、実行する特定の動作を選択する関数に渡されるパラメーターがある場合は、さらに段階的な改良が必要です。; この機能を小さな機能に分割すると、より高度にまとまりのある機能が生成されます。

つまり、非常にまとまりのある関数です?

それは一つのことと一つのことだけを行う機能です。

説明どおりに渡されたパラメーターの問題。, 関数は2つ以上のことを行っているということです。; ブールパラメータの状態に応じて、ユーザーのアクセス権を確認する場合としない場合があります。, 次に、その決定ツリーに応じて、機能の一部を実行します。

アクセス制御の懸念をタスク、アクション、またはコマンドの懸念から分離することをお勧めします。

すでに述べたように、これらの懸念の絡み合いはずれているようです。

したがって、コヒースネスの概念は、問題の機能が高度にまとまりがなく、コードをリファクタリングして一連のよりまとまりのある機能を生成できることを識別するのに役立ちます。

したがって、質問を再言できます。行動選択パラメータの通過は、問題をどのように改善するかを避けるのが最善であることに私たち全員が同意することを考えると、?

パラメータを完全に削除します。 テストでもアクセス制御をオフにできるかどうかは、潜在的なセキュリティリスクです。 [tag:stub]または[tag:mock]のいずれかのテスト目的で、アクセス許可シナリオとアクセス拒否シナリオの両方をテストするためのアクセスチェック。。

参照:結束(コンピューターサイエンス)。

これは必ずしも間違っているわけではありませんが、コード臭を表すことができます。

ブールパラメータに関して回避すべき基本的なシナリオは次のとおりです。

。

public void foo(boolean flag) {
    doThis();
    if (flag)
        doThat();
}

次に、電話をかけるときは、通常、必要な正確な動作に応じて「foo(false)」と「foo(true)」を呼び出します。

これは、結束力が悪い場合があるため、本当に問題です。 あなたは本当に必要ではない方法間の依存関係を作成しています。。

この場合、あなたがしなければならないことは、「doThis」と「doThat」を個別のパブリックメソッドとして残し、次に実行することです。

doThis();
doThat();

または。

doThis();

このようにして、カップリングを作成せずに、正しい決定を呼び出し元に任せます(まるでブールパラメーターを渡しているかのようです)。

もちろん、すべてのブールパラメータがこのように悪い方法で使用されているわけではありませんが、それは間違いなくコードのにおいであり、ソースコードでそれがたくさん見られた場合に疑わしいと思うのは正しいことです。

これは、私が書いた例に基づいてこの問題を解決する方法のほんの一例です。 別のアプローチが必要になる他のケースがあります。

同じ考えをさらに詳しく説明する良いMartin Fowlerの記事があります。

PS:2つの単純なメソッドを呼び出す代わりにメソッド fooがより複雑な実装を持っていた場合、実行する必要があるのは小さなリファクタリング抽出メソッドを適用することだけなので、結果のコードは fooの実装に似ています。書いた。

まず、プログラミングは芸術ほど科学ではありません。 したがって、プログラムする「間違った」方法と「正しい」方法はほとんどありません。 ほとんどのコーディング標準は、一部のプログラマーが有用であると考える単なる「好み」です。しかし、最終的にはかなり恣意的です。 したがって、私はパラメータの選択自体を「間違っている」とラベル付けすることは決してありません。確かに、ブールパラメータほど一般的で有用なものではありません。 状態をカプセル化するために「ブール」(または「int」)を使用することは、多くの場合完全に正当化できます。

&によるコーディングの決定大きな、主にパフォーマンスと保守性に基づいている必要があります。 パフォーマンスが危機に瀕していない場合(そして、それがあなたの例でどのようになるか想像できません)、次の考慮事項は次のとおりです。これは、私(または将来の編集者)が維持するのがいかに簡単か? それは直感的で理解可能ですか?? 分離されていますか?? チェーン関数呼び出しの例は、実際にはこの点で潜在的にもろく見えます。「bIsUp」を「bIsDown」に変更する場合、コード内の他の場所もいくつ変更する必要があります? また、あなたのパラメーターリストは膨れ上がっています? 関数に17個のパラメーターがある場合、読みやすさが問題となり、オブジェクト指向アーキテクチャの利点を理解しているかどうかを再検討する必要があります。

Robert C Martins Clean code article は、ブーリアン引数はメソッドが複数のことをすることを示すので、可能な限り排除すべきであると述べていると思います。 メソッドは1つのことを1つだけ行うべきだというのが彼のモットーの1つだと思います。

ここで最も重要なことは実用的であることだと思います。

ブール値で動作全体が決まったら、2番目の方法を作成します。

ブール値で中央に少ししか動作が決定されない場合は、コードの重複を減らすために、ブール値を一つに保つことをお勧めします。 可能な場合は、メソッドを3つに分割することもできます。ブールオプションの2つの呼び出しメソッドと、作業の大部分を実行する1つの呼び出しメソッド。

例:

。

private void FooInternal(bool flag)
{
  //do work
}

public void Foo1()
{
  FooInternal(true);
}

public void Foo2()
{
  FooInternal(false);
}

もちろん、実際には、これらの両極端の間に常にポイントがあります。 通常、私は正しいと感じるものだけを使いますが、コードの重複が少ないという側面で誤りを犯すことを好みます。

私は、不変のインスタンスを返すビルダーメソッドを通じて動作をカスタマイズするアプローチが好きです。 Guava Splitterの使用方法は次のとおりです。

。

private static final Splitter MY_SPLITTER = Splitter.on(',')
       .trimResults()
       .omitEmptyStrings();

MY_SPLITTER.split("one,,,,  two,three");

これの利点は次のとおりです。

優れた読みやすさ。 構成と明確な分離. アクションメソッド。 オブジェクトとは何か、オブジェクトが何をすべきか、すべきでないかについて考えさせることにより、結束を促進します。 この場合、それは「スプリッター」です。 someVagelyStringRelatedOperation(List< Entity> myEntities)を Splitterと呼ばれるクラスに配置することは決してありませんが、 StringUtilsクラスの静的メソッドとして配置することを考えます。 インスタンスは事前に構成されているため、依存関係から注入が容易になります。 クライアントは、正しい動作を取得するためにメソッドに「true」または「false」を渡すかどうかを心配する必要はありません。

TL; DR:ブール引数を使用しないでください。

以下を参照してくださいなぜ悪いのか、それらを置き換える方法(太字)。

---。

ブールの議論は読みにくく、維持するのが困難です。 主な問題は、引数が指定されているメソッドシグネチャを読み取るときに、目的が一般的に明確であることです。 ただし、ほとんどの言語では、通常、パラメータの命名は必要ありません。 したがって、[RSACryptoServiceProvider#encrypt(Byte []、Boolean)](https://msdn.microsoft.com/en-us/library/f17a0e2k(v=vs.110)のようなアンチパターンがあります。 aspx)ブールパラメーターは、使用する暗号化関数の種類を決定します.

したがって、次のような呼び出しが表示されます。

rsaProvider.encrypt(data, true);

ここで、読者はメソッドの署名を検索して、地獄の「真実」が実際に何を意味するかを判断する必要があります。 整数を渡すことももちろん悪いことです。

rsaProvider.encrypt(data, 1);

同じくらい、またはむしろ、少しだけ教えてくれます。 整数に使用する定数を定義した場合でも、関数のユーザーは単に定数を無視して文字値を使い続けることができます。

これを解決する最良の方法は、列挙を使用することです。 OAEPまたは PKCS1_V1_5の2つの値を持つ列挙型 RSAPaddingを渡す必要がある場合は、すぐにコードを読み取ることができます。

rsaProvider.encrypt(data, RSAPadding.OAEP);

---。

ブール値を持つことができるのは2つだけです。 つまり、3番目のオプションがある場合は、署名を改名する必要があります。 一般に、下位互換性が問題である場合、これは簡単に実行できないため、パブリッククラスを別のパブリックメソッドで拡張する必要があります。 これは、MicrosoftがRSACryptoServiceProvider#encrypt(Byte []、RSAEncryptionPadding)代わりに、列挙(または少なくともa.

パラメータ自体をパラメータ化する必要がある場合は、フルオブジェクトまたはインターフェイスをパラメータとして使用する方が簡単な場合があります。 上記の例では、OAEPパディング自体をハッシュ値でパラメーター化して内部で使用できます。 現在、6つのSHA-2ハッシュアルゴリズムと4つのSHA-3ハッシュアルゴリズムがあるため、パラメーターではなく単一の列挙のみを使用すると、列挙値の数が爆発する可能性があります(これはおそらくMicrosoftが次に調べるものです) )。

---。

ブールパラメータは、メソッドまたはクラスが適切に設計されていないことを示している場合もあります。 上記の例と同様に、.NET以外の暗号化ライブラリでは、メソッドシグネチャでパディングフラグがまったく使用されません。

---。

私が好きなほとんどすべてのソフトウェアの教祖は、ブール論の議論に対して警告します。 たとえば、ジョシュア・ブロックは、高く評価されている本「効果的なジャワ」で彼らに警告しています。 一般に、それらは単に使用されるべきではありません。 理解しやすいパラメータが1つある場合、それらを使用できると主張できます。 しかし、それでも: Bit.set(boolean)は、 2つのメソッドを使用して実装する方が良いでしょう: Bit.set()およびBit.unset()。

---。

コードを直接リファクタリングできない場合は、定数**を定義して、少なくとも定数を読みやすくすることができます。

const boolean ENCRYPT = true;
const boolean DECRYPT = false;

...

cipher.init(key, ENCRYPT);

:よりもはるかに読みやすいです。

cipher.init(key, true);

あなたがむしろ持っているとしても:

cipher.initForEncryption(key);
cipher.initForDecryption(key);

代わりに。

間違いなくコード臭い。 単一責任原則に違反していない場合は、おそらく[Tell、Do n't Ask](http://pragprog.com/articles/tell -dont-ask)。 考慮:

-接続詞を使用せずにメソッドが実行することを要約できますか(および、しかしなど)。)? -簡単に[別の小さなメソッドにグループ化]できるメソッドのサブパートはありますか(http://brianwill.net/blog/2007/03/28/howto-decompose-your-code-into-functions/)? (これには、ローカル変数の複雑な作成が含まれます。)。 -フラグがパラメーターを呼び出す方法を決定する場合、パラメーターはやりすぎです? -このメソッドはパラメータの1つに属していますか? ([getters / setters](http://www.javaworld.com/javaworld/を使用して、パラメーターで[インスタンス変数を切り離す](http://www.idinews.com/quasiClass.pdf)に気づくかもしれません。 jw-09-2003 / jw-0905-toolbox.html?page = 1)。)。

これら2つの原則のいずれかに違反していないことが判明した場合は、引き続き列挙を使用する必要があります。 ブールフラグは、マジックナンバーに相当するブール値です。 foo(false)は bar(42)と同じくらい意味があります。 Enumsは戦略パターンに役立ち、別の戦略を追加できる柔軟性があります。 (適切に名前を付けるを覚えておいてください。)。

あなたの特定の例は特に私を悩ませます。 なぜこの旗は非常に多くの方法を通過するのですか?? これは、パラメータをサブクラスに分割するにする必要があるようです。

名前付きパラメータについて誰も言及していないことに驚いています。

私がブールフラグで見る問題は、読みやすさを損なうことです。 たとえば、「true」は何を入力します。

。

myObject.UpdateTimestamps(true);

行う? わからない。 しかし、どうですか:

myObject.UpdateTimestamps(saveChanges: true);

これで、渡しているパラメーターが何を意味するのかが明確になりました。関数に変更を保存するように指示しています。 この場合、クラスが非公開の場合、ブールパラメータは問題ないと思います。

---。

もちろん、クラスのユーザーに名前付きパラメーターの使用を強制することはできません。 このため、デフォルトのケースの共通点に応じて、「enum」または2つの別々のメソッドが通常望ましい。 これはまさに.Netが行うことです。

//Enum used
double a = Math.Round(b, MidpointRounding.AwayFromZero);

//Two separate methods used
IEnumerable ascendingList = c.OrderBy(o => o.Key);
IEnumerable descendingList = c.OrderByDescending(o => o.Key); 

私は、ブールパラメータを使用してパフォーマンスを決定しないというすべての懸念に同意します。改善、読みやすさ、信頼性、複雑さの低下、不十分なカプセル化によるリスクの低下&維持可能性を備えた結束と総所有コストの削減。

70年代半ばにハードウェアの設計を開始しました。これは現在SCADA(監視制御とデータ収集)と呼ばれ、これらはEPROMのマシンコードがマクロリモートコントロールを実行し、高速データを収集する微調整ハードウェアでした。

ロジックは Mealey &と呼ばれます。 ムーア現在呼び出すマシン Finite State Machines 。 これらは、有限の実行時間を持つリアルタイムマシンであり、目的を果たすためにショートカットを行う必要がある場合を除き、上記と同じルールで行う必要があります。

データは同期していますが、コマンドは非同期であり、コマンドロジックはメモリレスブールロジックに従いますが、以前、現在、および目的の次の状態のメモリに基づく順次コマンドを使用します。 それが最も効率的な機械言語(64kBのみ)で機能するために、ヒューリスティックIBM HIPO方式ですべてのプロセスを定義するように細心の注意が払われました。 これは、ブール変数を渡してインデックス付きブランチを実行することを意味する場合があります。

しかし、多くのメモリとOOKの使いやすさを備えたエンカプセルは、今日では不可欠な要素ですが、コードがリアルタイムでバイトでカウントされ、SCADAマシンコードがカウントされた場合のペナルティです。.。

何が悪いのかを明確に述べることはできません。

コードのにおいのように見え、コードのにおいのように感じ、コードのにおいのようなにおいがする場合、それはおそらくコードのにおいです。

あなたがしたいことは:です。

1)副作用のある方法は避けてください。

2)必要な状態を中央の正式な状態マシンで処理します(thisなど)。

それは必ずしも間違っているわけではありませんが、「ユーザー」の一部の属性に応じたアクションの具体的な例では、フラグではなくユーザーへの参照を通過します。

これは、いくつかの方法で明確化し、役立ちます。

呼び出しステートメントを読む人は誰でも、ユーザーによって結果が変化することを認識します。

最終的に呼び出される関数では、ユーザーの属性にアクセスできるため、より複雑なビジネスルールを簡単に実装できます。

「チェーン」の1つの関数/メソッドがユーザー属性に応じて異なる何かを行う場合、ユーザー属性への同様の依存関係が「チェーン」の他のいくつかのメソッドに導入される可能性が非常に高いです。

ほとんどの場合、私はこの悪いコーディングを検討します。 しかし、これが良い習慣になるかもしれない2つのケースを考えることができます。 なぜそれが悪いのかをすでに言っている多くの答えがあるので、私はそれが良いかもしれないときに2回提供します:

1つ目は、シーケンスの各呼び出しがそれ自体で意味がある場合です。 呼び出しコード_might_をtrueからfalseまたはfalseからtrueに変更する場合、または_or_呼び出されたメソッド_might_を変更して、ブールパラメータを渡すのではなく直接使用する場合、それは理にかなっています。 このような呼び出しが10回連続して行われる可能性は小さいですが、発生する可能性があり、発生した場合、それは優れたプログラミングの実践になります。

2番目のケースは、ブール値を扱っていることを考えると、少しストレッチです。 ただし、プログラムに複数のスレッドまたはイベントがある場合、パラメーターを渡すことは、スレッド/イベント固有のデータを追跡する最も簡単な方法です。 たとえば、プログラムは2つ以上のソケットから入力を取得できます。 あるソケットで実行されているコードは警告メッセージを生成する必要がある場合があります。別のソケットで実行されているコードは生成されない場合があります。 その場合(の一種)は、非常に高いレベルのブールセットが警告メッセージが生成される可能性のある場所への多くのメソッド呼び出しを通過するのに意味があります。 複数のスレッドまたはインターリーブされたイベントにはそれぞれ独自の値が必要になるため、データを(非常に困難を除いて)あらゆる種類のグローバルに保存することはできません。

確かに、後者の場合、おそらくブール値のみのコンテンツであるクラス/構造を作成し、代わりに_that_を渡します。 たとえば、警告メッセージを送信するために_where_のように、他のフィールドが必要になることはほぼ確実です。

コンテキストは重要です。このようなメソッドは、iOSではかなり一般的です。よく使われる例として、UINavigationControllerには -pushViewController:animated: というメソッドがあり、animated というパラメータはBOOLである。このメソッドは基本的に同じ機能を実行しますが、YESを渡すとビューコントローラから別のビューコントローラへの遷移をアニメーション化し、NOを渡すとアニメーション化しないようにします。アニメーションを使用するかどうかを決定するために、このメソッドの代わりに2つのメソッドを提供しなければならないのは愚かなことです。

Objective-Cでは、メソッド名の構文がCやJavaのような言語よりも各パラメータに多くのコンテキストを与えてくれるので、このようなことを正当化するのは簡単かもしれませんね。しかし、1つのパラメータを受け取るメソッドであれば、ブール値を受け取ることは容易であり、まだ意味があると私は思います：

file.saveWithEncryption(false);    // is there any doubt about the meaning of 'false' here?