2

私はちょうど見つけました...もう一度...次のようにリアルタイムの浪費バグ

for (int i = 0; i < length; i++)
{ //...Lots of code 
    for (int j = 0; i < length; j++)
    {
        //...Lots of code 
    }
}

あなたはjであるべきである内側のiの真正面に気づきましたか？私もそうではありませんでした。それで、これから私は以下を使用するつもりです：

for (int i = 0; i < length; i++)
{
    for (int i1 = 0; i1 < length; i1++)
    {
    }
}

内側と外側のwhileループとforループのヒントは何ですか？

編集：貴重な回答をありがとう。提案されたヒントの簡単な要約を以下に示します。

  • インデックス変数に意味のある変数名を使用します（代わりにSomeObjCollectionLengthを使用します）
  • 内側のループの内容を別のメソッドに配置し、外側のループからそのメソッドを呼び出します
  • 外側のループと内側のループの間の管理できない量のコード行は、コードの臭いの強いシグナルです
  • コピーして貼り付けたり急いだりしないでください。インデックス変数は注意して記述してください。

LBushkinによる要約を次のように確認することをお勧めします

  • 可能な限りforeachとイテレータを使用する
  • ループに入る直前に変数を初期化します
  • 各ループに1つの機能のみを実行させます。単一のループで責任を混在させないでください
  • 可能であれば、ループを十分に短くして、一度にすべてを表示できるようにします
4

12 に答える 12

7

i & j (またはその他の単一文字変数) をインデックス名として使用しないでください。適切な名前を使用すれば、この種の問題に陥ることはありません。

于 2009-06-18T09:33:18.140 に答える
6

最も単純でクリーンな解決策の 1 つは、内側のループの内容をメソッドに配置して、次のようにすることです。

for (int i = 0; i < length; i++)
{
    DoSomething();
}

private void DoSomething(int outerValue)
{
    for (int i = 0; i < length; i++)
    {
        // Do something else
    }

}
于 2009-06-18T09:32:45.910 に答える
4

より良いループコードを書くための私の一番のアドバイス (順不同) (これの多くは優れた本Code Completeからのものです):

  1. ループの複数の出口点は避けてください。
  2. continue/break は慎重に使用してください。
  3. 可能であれば、ネストされたループを個別のルーチンにリファクタリングします。
  4. 意味のある変数名を使用して、ネストされたループを読みやすくします。
  5. for(i=...) ループではなく、可能であれば foreach() ループを使用します。
  6. ループには 1 か所からのみ入ります。goto でループに飛び込まないでください。これまで。
  7. ループの直前に初期化コードを置きます。
  8. ループの初期化ステートメントは、関連するループとともに保持します。
  9. ネストされていないループ間で変数を再利用することは避けてください。10. ループ インデックス変数のスコープをループ自体に制限します。
  10. 無限ループには、for(;;) ではなく while(true) を使用します。
  11. ブロック構文 ('{' や '}' など) を提供する言語では、ループのステートメントを囲むためにインデントするのではなく、それらを使用します。はい、単線ループでも可能です。
  12. 空のループは避けてください。
  13. ハウスキーピングの雑用をループの途中に配置するのは避け、代わりに最初または最後に配置します。
  14. 各ループが 1 つの機能のみを実行するようにします。1 つのループ内で責任を混在させないようにします。
  15. ループ終了条件を明確にします。
  16. for() ループのループ インデックス変数を使用して終了させないでください。
  17. ループ インデクサーの最終値に依存するコードは避けてください。
  18. 複雑なループで安全カウンターを使用することを検討してください。安全カウンターをチェックして、ループの実行回数が多すぎたり少なすぎたりしないようにすることができます。
  19. 可能であれば、break ステートメントを使用して while ループを終了します。
  20. 可能であれば、一度にすべてを表示できるように、ループを十分に短くしてください。
于 2009-06-18T20:38:22.503 に答える
4

私にとって、ここでの「コードの匂い」は「たくさんのコード」です。

ループ内のコード量が特に多い場合、内側のループと外側のループの間の距離は、互いの正確性を比較する可能性が低いことを意味します。

確かに、内側のループの開始点を単独で見ると、問題に注意を向けることができますが、主要な構造をコードのできるだけ小さなセクションにすることで、脳が消化しにくくなります。

メイン構造のサイズを縮小するために、「大量のコード」セクションを個別の関数/メソッドに抽出することは可能かもしれませんが、これは常に実用的であるとは限りません。

また、「i1」は変数名の特に良い選択ではないと思います。「i2」、「i3」などを助長する傾向があり、実際には理解可能なコードにつながらないからです。おそらく、すべてのループ変数をより意味のあるものに置き換えると、コードがわかりやすくなり、元のエラーの可能性が減ります。

于 2009-06-18T09:35:45.217 に答える
3

これはコピペの間違いです。コピペは避けてください。

あなたの解決策に関しては、それほど良くはありません。間違いは、大量のコードの間をすり抜けてしまう可能性があります。ループの一時変数にも意味のある名前を使用する傾向があります。

于 2009-06-18T09:30:53.527 に答える
2

VSでIDEを活用して、これを使用してみてください：http://msdn.microsoft.com/en-us/library/z4c5cc9b(VS.80).aspx

サンプル: forと入力し、 Tab Tabを続けて押します

于 2009-06-18T09:40:35.973 に答える
2

私は賢く、「最初から正しく書くだけだ」と言うためにここに来ました。しかし、私はあなたの例を見ました。

そのような入れ子になったループが必要な場合、私の唯一の解決策は、コードを書くときに注意深く考えることです。

可能であれば、反復子と for each ループを使用すると便利です。

また、提案されたソリューションがどのように改善されるかわかりません。そして、見た目も美しくありません。

于 2009-06-18T09:42:21.990 に答える
2

まず、ループ本体のサイズを小さくします。つまり、要素を別の関数に移動します。一般に、画面に収まりきらないほど長い関数を使用することはお勧めできません。そのため、ループはさらに小さくする必要があります。

次に、このような場合は意味のある変数名を使用してください。i と j は、数行のコードを含む単純なループでのみ使用します。たとえば、2 次元配列を使用している場合、"col" と "row" の方がはるかに意味があり、コードが読みやすくなり (「どれがどれだった?」)、このような間違いを見つけやすくなります。

于 2009-06-18T09:47:52.620 に答える
1

このような問題には特別な注意を払う必要があります。これに対する特効薬はありません。「より良い命名」を使用しても、これがネストされたループのN番目または(N + M)番目のレベルであるかどうかを時々見失い、エラーを起こすことを提案します。

ネストされたループが必要な場合は、慎重に記述してください。外側のループ本体を関数に抽出することで回避できれば、インデックスの誤用を防ぐことができます。

于 2009-06-18T09:30:00.143 に答える
0

これと同様に、Steve McConnell のCode Completeには優れたアドバイスがあります。優れたループ コードの作成について彼が述べていることを読むのは、時間の無駄ではないでしょう。私は自分の本を手元に置いていませんが、本全体を読む価値はあります。

于 2009-06-18T09:40:48.453 に答える
0

本当に必要な場合は、トランジェント ループ カウンターに 'ii' と 'jj' を使用します。これらは、'i' と 'j' よりも検索しやすく、上記のような例で簡単に見つけることができます。より良いものにするために、実際に実際の変数名を使用できます。文字列をループしている場合は、それを characterIndex などと呼ぶことができます。より多くのタイピングが必要ですが、それ自体が文書化され、後であいまいな問題をデバッグする時間を節約できます。

数値カウンターを避け、コレクションに対して名前付き反復子を使用することをお勧めします。私の意見では、それらは意図をより明確にします。

最後に、可能であれば、ループを完全になくすのが良いでしょう: Boost::Foreachは C++ でこれを行う 1 つの方法ですが、私は一般的に Python などの言語を使用することを好みます。インデックス値または反復子をインクリメントする必要があります。

于 2009-06-18T09:34:30.773 に答える
0

より宣言的なループ構成を使用するようにしてください。iたとえば、インデックス (これらの および)が実際には必要なくj、プログラミング環境で許可されている場合は、foreachコンストラクトを使用してコレクションを反復処理できます。

于 2009-06-18T09:36:36.653 に答える