java - ラベル付きループのリファクタリング

Question

ラベル付きのブレーク/コンティニューはここでは完全に「nono」であると確信した後、コードからラベルを削除するための支援が必要です。

同じ長さの正方行列とベクトルがあります。ベクトルには、マトリックス内の値に応じて、ループ内でベクトルが変更されるいくつかの値がすでに含まれています。

コードフラグメントが基本的に理解できることを願っています…

vectorLoop:
for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) ) continue vectorLoop;

    matrixLoop:
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue matrixLoop;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) continue vectorLoop;
    }
    setValueInVector( v, idx );
}

ラベルのない、より読みやすい/より良いバージョンがあることを私に納得させてください。

score 34 · Accepted Answer

これまでに提示されたソリューションを見てください。

それらはすべて、アルゴリズム自体ではなく、コードのメカニズムにより多くのコードを費やすという点で、元のコードよりも読みにくく見えます。
それらのいくつかは壊れているか、編集される前のものでした。最も気の毒なのは、人々がラベルなしでコードを書く方法について非常に熱心に考えなければならず、何も壊さないという事実です。
同じテストを2回実行するとパフォーマンスが低下するものもありますが、これは必ずしも些細なことではありません。これに代わる方法は、醜いラウンドブール値を格納して渡すことです。
コードの関連部分をメソッドにリファクタリングすることは、事実上何もしません。コードがファイル内でどのように配置されるかを再配置しますが、実行方法には影響しません。

これらすべてから、少なくともこの質問のフレーズの場合、ラベルは正しい解決策であり、リファクタリングする必要はないと私は信じています。確かに、ラベルが正しく使用されておらず、リファクタリングする必要がある場合があります。私はそれが破られないルールとして扱われるべきではないと思います。

score 1 · Accepted Answer

@ Sadie：

それらはすべて、アルゴリズム自体ではなく、コードのメカニズムにより多くのコードを費やすという点で、元のコードよりも読みにくく見えます。

アルゴリズムの外部で2番目のループを外部化することは、必ずしも読みにくくなるとは限りません。メソッド名を適切に選択すると、読みやすさが向上します。

それらのいくつかは壊れているか、編集される前のものでした。最も気の毒なのは、人々がラベルなしでコードを書く方法について非常に熱心に考えなければならず、何も壊さないという事実です。

私は別の見方をしています。元のアルゴリズムの動作を理解するのが難しいため、それらのいくつかは壊れています。

同じテストを2回実行するとパフォーマンスが低下するものもありますが、これは必ずしも些細なことではありません。これに代わる方法は、醜いラウンドブール値を格納して渡すことです。

パフォーマンスの低下は軽微です。ただし、テストを2回実行することは良い解決策ではないことに同意します。

コードの関連部分をメソッドにリファクタリングすることは、事実上何もしません。コードがファイル内でどのように配置されるかを再配置しますが、実行方法には影響しません。

要点がわかりません。うん、それは振る舞いを変えない、例えば...リファクタリング？

確かに、ラベルが正しく使用されておらず、リファクタリングする必要がある場合があります。私はそれが破られないルールとして扱われるべきではないと思います。

全くもって同じ意見です。しかし、ご指摘のとおり、この例をリファクタリングする際に問題が発生する人もいます。最初の例が読みやすくても、維持するのは難しいです。

score 1 · Accepted Answer

@ニコラス

それらのいくつかは壊れているか、編集される前のものでした。最も気の毒なのは、人々がラベルなしでコードを書く方法について非常に熱心に考えなければならず、何も壊さないという事実です。

私は別の見方をしています。元のアルゴリズムの動作を理解するのが難しいため、それらのいくつかは壊れています。

主観的だとは思いますが、元のアルゴリズムを読むのに問題はありません。提案された代替品よりも短く、明確です。

このスレッドのすべてのリファクタリングは、他の言語機能を使用してラベルの動作をエミュレートします。まるで、ラベルのない言語にコードを移植しているかのように。

score 1 · Accepted Answer

簡単に、私のいい男。

for( int idx = 0; idx < vectorLength; idx++) {
  if( conditionAtVectorPosition( v, idx ) ) continue;

  for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
    if( anotherConditionAtVector( v, rowIdx ) ) continue;
    if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) break;
  }
  if( !conditionAtMatrixRowCol( m, rowIdx, idx ) )
    setValueInVector( v, idx );
}

編集：あなたはアンダースです。それも考慮に入れてソリューションを編集しました。

score 1 · Accepted Answer

@Patrickは、setValueInVector（v、idx）;を呼び出すことを想定しています。2番目のループの終わりにOKです。コードが論理的に同一である場合は、次のように書き直す必要があります。

for（int idx = 0; idx

score 1 · Accepted Answer

同じテストを 2 回実行するというパフォーマンス上のペナルティが伴うものもありますが、これは必ずしも些細なことではありません。それに代わる方法は、ラウンドブール値を保存して渡すことですが、これは見苦しくなります。
パフォーマンスの低下は軽微です。ただし、テストを 2 回実行することは適切な解決策ではないことに同意します。

問題は、アルゴリズムを最適化する方法ではなく、ラベルを削除する方法だと思います。元の投稿者はラベルなしで「continue」および「break」キーワードを使用する方法を知らなかったように見えましたが、もちろん、私の仮定は間違っている可能性があります。

パフォーマンスに関しては、投稿は他の関数の実装に関する情報を提供していないため、コンパイラによってインライン化された単純な計算で構成されるものとして、FTP 経由で結果をダウンロードすることもできます。

そうは言っても、理論的には、同じテストを 2 回行うことは最適ではありません。

編集: もう一度考えてみると、この例は実際にはラベルの恐ろしい使い方ではありません。「gotoはノーノー」であることに同意しますが、このようなコードのせいではありません。ここでラベルを使用しても、実際にはコードの可読性に大きな影響はありません。もちろん、それらは必須ではなく、簡単に省略できますが、単に「ラベルを使用するのは悪い」という理由で使用しないことは、この場合には適切な議論ではありません。結局のところ、他の人がすでにコメントしているように、ラベルを削除してもコードが読みやすくなるわけではありません。

score 1 · Accepted Answer

ラベル付けされたループは非常に珍しいので、自分に合ったラベル付け方法を選択できると思います。そこにあるものは、継続の意図を完全に明確にします。

元の質問でループをリファクタリングすることを提案するために起訴を主導し、問題のコードを見て、非常に読みやすいループがあると思います。

私が想像していたのは、非常に異なるコードの塊でした。実際の例を上げてみると、思っていたよりもはるかにきれいであることがわかります。

誤解をお詫び申し上げます。

score 1 · Accepted Answer

この質問は、アルゴリズムの最適化に関するものではありませんでしたが、とにかくありがとうございました ;-)

私がそれを書いた時点で、私はラベル付きの継続を読みやすい解決策と考えていました。

Java のラベルの規則 (ラベルがすべて大文字かどうか) についてSO に質問しました。

基本的に、すべての回答は「それらを使用しないでください-常により良い方法があります!リファクタリング!」と言っていました。そこで、この質問を投稿して、より読みやすい (したがってより良い?) ソリューションを求めました。

これまでのところ、私はこれまでに提示された代替案に完全には納得していません.

誤解しないでください。ほとんどの場合、ラベルは悪です。

しかし、私の場合、条件付きテストは非常に単純で、アルゴリズムは数学論文から引用されているため、近い将来変更されない可能性が非常に高いです。したがって、checkMatrixAtRow(x) などの別のメソッドにスクロールするよりも、関連するすべての部分を一度に表示することを好みます。

特により複雑な数学的アルゴリズムでは、「良い」関数名を見つけるのはかなり難しいと思いますが、それはまた別の問題だと思います

score 1 · Accepted Answer

あなたのコードを読むことから。

conditionAtVectorPosition で無効なベクトル位置を削除してから、anotherConditionAtVector で無効な行を削除していることに気付きました。
idx の値が何であれ、anotherConditionAtVector は行インデックスのみに依存するため、anotherConditionAtVector で行をチェックすることは冗長であるようです (anotherConditionAtVector には副作用がないと仮定します)。

だからあなたはこれを行うことができます：

最初に conditionAtVectorPosition を使用して有効な位置を取得します (これらは有効な列です)。
次に、anotherConditionAtVector を使用して有効な行を取得します。
最後に、有効な列と行を使用して conditionAtMatrixRowCol を使用します。

これが役立つことを願っています。

score 0 · Accepted Answer

これはあなたのために働きますか？内側のループをメソッドCheckedEntireMatrixに抽出しました（私よりも名前を付けることができます）-また、私のJavaは少し錆びています..しかし、メッセージが伝わると思います

for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) 
    || !CheckedEntireMatrix(v)) continue;

    setValueInVector( v, idx );
}

private bool CheckedEntireMatrix(Vector v)
{
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }   
    return true;
}

score 0 · Accepted Answer

ギシュの考えは正しい .

for( int idx = 0; idx < vectorLength; idx++) {
    if (!conditionAtVectorPosition( v, idx ) 
        && checkedRow(v, idx))
         setValueInVector( v, idx );
}

private boolean checkedRow(Vector v, int idx) {
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }  
    return true;
}

score 0 · Accepted Answer

最初の続きがよくわかりません。ギシュウをコピーして次のように書きます（間違いがあればごめんなさい）：

for( int idx = 0; idx < vectorLength; idx++) {
    if( !conditionAtVectorPosition( v, idx ) && CheckedEntireMatrix(v))
        setValueInVector( v, idx );
}

inline bool CheckedEntireMatrix(Vector v) {
    for(rowIdx = 0; rowIdx < n; rowIdx++)
        if ( !anotherConditionAtVector(v,rowIdx) && conditionAtMatrixRowCol(m,rowIdx,idx) ) 
            return false;
    return true;
}

java - ラベル付きループのリファクタリング

12 に答える 12

Related

Reference