2011-06-20 13 views
4
#include <iostream> 
using namespace std; 

int main() 
{ 
    int range = 20; 
    int totalCombinations = 0; 

    for(int i=1; i<=range-2; i++) 
    { 
     if(range>i) 
     { 
      for(int j=1; j<=range-1; j++) 
       if(j>i) 
       { 
        for(int k=1; k<=range-1; k++) 
         if(k>j) 
         { 
          for(int l=1; l<=range-1; l++) 
           if(l>k) 
           { 
            for(int m=1; m<=range-1; m++) 
             if(m>l) 
             { 
              for(int f=1; f<=range; f++) 
               if(f>m) 
               { 
                cout << " " <<i<< " " <<j<< " " <<k<< " " <<l<< " " <<m<< " " <<f; 
                cin.get(); //pause 
                totalCombinations++; 
               } 
             } 
           } 
         } 
       } 
     } 
    } 
    cout << "TotalCombinations:" << totalCombinations; 
} 
+0

は誤ってCとタグ付けされています。 – GENRO

答えて

4

あなたが行うことができます最初の事はusing continueです:

for(int i=1; i<=range-2; i++) { 
{ 
    if(range<=i) { 
     continue; 
    } 

    for(int j=1; j<=range-1; j++) { 
     if(j<=i) { 
      continue; 
     } 
     //etc for all inner loops 
    } 
} 

この大幅に読みやすさを向上させるIMOネストを削減します。

+0

これを行うことができます。または、完全に冗長なIFテストを排除することができます(私が指定したように...)。 +1はこれが問題をより一般的に解決するためですが、この特定のケースではより良い解決策が存在すると思います。 –

+0

コードを読むことができず、維持することができません。最初のことは、Billy ONealが言ったことです:不要な 'if'を取り除くことです。 (それが価値あるものであれば、最初のものはいつも偽になるでしょう。) –

+0

@ James Kanze:これらのチェックは不要ですが、私が提案するリファクタリングは正式に行うことができ、これらのチェックが不要であることに気付くのが簡単になります。 – sharptooth

4
if(range>i) 

理由だけrangeiを開始し、問題を回避しませんか?ああ、私はそれを後方に持っていましたが、ポイントは立っています - これを簡単にリファクタリングしてforの条件の一部にすることができます。特別な条件の必要はありません。

if(j>i) 

理由だけijを起動しませんか?半分のあなたのネスティングを取り除く

...(他の二つのループのために繰り返します)

。限り、ル​​ープ自体が行く、私はそれらの抽出メソッドを使用することをお勧めします。

+0

+1:さらに、 'i'は常に' range'より小さくなります。 –

0

リファクタリングと同じ方法です。まず、 が何をしているのか把握しなければなりません。この場合、多くのテストは無関係であり、 の各ループは基本的に同じことを行います。より一般的な問題の具体的なケース(非常にうっすら)を解決しました。 を解決すると、問題の一般的なアルゴリズムはより洗練された簡単な ソリューションとなり、より一般的なソリューションとなります。このような何か:

class Combin 
{ 
    int m; 
    int n; 
    int total; 
    std::vector<int> values; 

    void calc(); 
    void dumpVector() const; 
public: 
    Combin(int m, int n) : m(m), n(n), total(0) {} 
    int operator()() { total = 0; values.clear(); calc(); return total; } 
}; 

void 
Combin::calc() 
{ 
    if (values.size() == m) { 
     dumpVector(); 
     ++ total; 
    } else { 
     values.push_back(values.empty() ? 0 : values.back() + 1); 
     int limit = n - (m - values.size()); 
     while (values.back() < limit) { 
      calc(); 
      ++ values.back(); 
     } 
     values.pop_back(); 
    } 
} 

void 
Combin::dumpVector() const 
{ 
    for (std::vector<int>::const_iterator iter = values.begin(); iter != values.end(); ++ iter) 
     std::cout << ' ' << *iter + 1; 
    std::cout << '\n'; 
} 

int main() 
{ 
    Combin c(6, 20); 
    std::cout << "TotalCombinations:" << c() << std::endl; 
    return 0; 
} 

上記でコメントの本当に価値がある唯一の事はcalclimitの計算 であり、それは本当にただの最適化です。 nを使用しても同じ結果が得られます(ただし、もう少し詳しく説明します)。あなたがあなたの元のバージョンでは、 ループの終了条件は、多かれ少なかれ任意であることに注意しましょう

:体系 仕事だろうか、limitのために私が使用して式をうまくできrangeを使用して( う各ループごとに異なる終了条件での結果。また

は、私のコードはCと C++でubiquiousある半分開いた間隔を使用しています。私は、あなたがそれらに慣れたら、あなたはずっと 簡単にそれらを見つけることだと思います理由について。

0

My C++ i錆びているので、C#の例を挙げましょう。任意の数のネストされたループは、次のようにただ1つに置き換えることができます。

public void ManyNestedLoopsTest() 
    { 
     var limits = new[] {2, 3, 4}; 
     var permutation = new[] {1, 1, 0}; 
     const int lastDigit = 2; 
     var digitToChange = lastDigit; 
     while(digitToChange >= 0) 
     { 
      if (permutation[digitToChange] < limits[digitToChange]) 
      { 
       permutation[digitToChange]++; 
       digitToChange = lastDigit; 
       PrintPermutation(permutation); 
       continue; 
      } 
      permutation[digitToChange--] = 1; 
     } 
    } 

    private void PrintPermutation(int[] permutation) 
    { 
     for(int i=0;i<3;i++) 
     { 
      Console.Write(permutation[i]); 
      Console.Write(" "); 
     } 
     Console.WriteLine(" "); 
    } 
関連する問題