ラベル付きループをリファクタリングして取り除く

StackOverflow https://stackoverflow.com/questions/15851

  •  08-06-2019
  •  | 
  •  

質問

ラベル付きのブレイク/コンティニューは完全に「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 );
}

ラベルのない、より読みやすく、より優れたバージョンがあることを納得してください。

役に立ちましたか？

解決

これまでに提示されたソリューションを見ると、次のようになります。

  • これらはすべて、アルゴリズム自体ではなくコードのメカニズムにより多くのコードを費やしているため、元のものよりも読みにくくなっています。

  • 一部は壊れているか、編集される前のものです。最もひどいのは、ラベルを付けずに、何も壊さずにコードを書く方法について、人々がかなり真剣に考えなければならないという事実です。

  • 一部のテストでは、同じテストを 2 回実行するというパフォーマンス上のペナルティが発生しますが、これは必ずしも簡単ではない場合があります。これに代わる方法は、ブール値を格納して渡すことですが、これは見苦しくなります。

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

これらすべてのことから、少なくともこの質問の表現の場合、ラベルは正しい解決策であり、リファクタリングして取り除く必要はないと私は信じています。確かに、ラベルが誤って使用され、リファクタリングして除去する必要がある場合があります。ただ、それを破ることのできないルールとして扱うべきではないと思います。

他のヒント

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

for( int idx = 0; idx

簡単です、私の良い人。

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 );
}

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

コードを読むことから。

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

したがって、次のようにすることができます。

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

これがお役に立てば幸いです。

@ニコラス

一部は壊れているか、編集される前のものです。ほとんどのことは、人々がラベルなしでコードを書く方法について非常に一生懸命に考えなければならないという事実です。

私は別の視点を持っています:それらのいくつかは、元のアルゴリズムの動作を把握するのが難しいために壊れています。

主観的なものであることは承知していますが、元のアルゴリズムを読むのに問題はありません。提案されている代替案よりも短く、明確です。

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

一部のテストでは、同じテストを 2 回実行するというパフォーマンス上のペナルティが発生しますが、これは必ずしも簡単ではない場合があります。これに代わる方法は、ブール値を格納して渡すことですが、これは見苦しくなります。
パフォーマンス上のペナルティは軽微です。ただし、テストを 2 回実行するのは良い解決策ではないことに私も同意します。

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

パフォーマンスに関して言えば、この投稿には他の関数の実装に関する情報がまったく記載されていないため、私が知っている限りでは、コンパイラーによってインライン化された単純な計算からなる結果を FTP 経由でダウンロードするのと同じかもしれません。

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

編集：よく考えてみると、この例は実際にはラベルのひどい使用法ではありません。私はそれに賛成だ 「後藤はダメだよ」, 、しかし、このようなコードのせいではありません。ここでのラベルの使用は、実際にはコードの読みやすさに大きな影響を与えません。もちろん、これらは必須ではなく、簡単に省略できますが、単に「ラベルを使用するのは悪い」という理由でラベルを使用しないのは、この場合良い議論ではありません。結局のところ、他の人がすでにコメントしているように、ラベルを削除してもコードはそれほど読みやすくなりません。

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

これを書いた時点では、ラベル付きの continue が読みやすいソリューションであると考えていました。

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

基本的にどの回答も「使用しないでください。より良い方法は常にあります。リファクタリングしてください！」そこで、より読みやすい（したがってより良い？）解決策を求めるためにこの質問を投稿しました。

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

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

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

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

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

元の質問でループのリファクタリングを提案するよう先頭に立ち、問題のコードを確認すると、非常に読みやすいループができたと思います。

私が想像していたものは、まったく異なるコードの塊でした。実際の例をアップしてみると、思っていたよりもはるかにクリーンであることがわかります。

誤解を招いてしまい申し訳ございません。

これは役に立ちますか?内部ループをメソッド 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;
}

ギシュの考えは正しい：

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;
}

最初のコンティニューがよくわかりません。私なら岐州を真似て次のように書きます（間違いがあればごめんなさい）。

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;
}

@セイディ:

これらはすべて、アルゴリズム自体ではなくコードのメカニズムにより多くのコードを費やしているため、元のものよりも読みにくくなっています。

2 番目のループをアルゴリズムの外側に外部化しても、必ずしも可読性が低下するわけではありません。メソッド名を適切に選択すると、読みやすさが向上します。

一部は壊れているか、編集される前のものです。最もひどいのは、ラベルを付けずに、何も壊さずにコードを書く方法について、人々がかなり真剣に考えなければならないという事実です。

私は別の視点を持っています:元のアルゴリズムの動作を理解するのが難しいため、それらの一部は壊れています。

一部のテストでは、同じテストを 2 回実行するというパフォーマンス上のペナルティが発生しますが、これは必ずしも簡単ではない場合があります。これに代わる方法は、ブール値を格納して渡すことですが、これは見苦しくなります。

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

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

要点がわかりません。はい、動作は変わりません...リファクタリング？

確かに、ラベルが誤って使用され、リファクタリングして除去する必要がある場合があります。ただ、それを破ることのできないルールとして扱うべきではないと思います。

全くもって同じ意見です。しかし、あなたが指摘したように、この例のリファクタリング中に問題を抱えている人もいます。たとえ最初の例が読みやすかったとしても、それを維持するのは困難です。

ライセンス： CC-BY-SA と 帰属
所属していません StackOverflow
scroll top