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

查看迄今为止提出的解决方案：

它们看起来都比原来的可读性差，因为它们涉及在代码机制而不是算法本身上花费更多的代码
其中一些已损坏，或者在编辑之前就已损坏。最糟糕的是，人们不得不非常努力地思考如何编写没有标签的代码并且不破坏任何东西。
有些会带来两次运行相同测试的性能损失，这可能并不总是微不足道的。另一种方法是存储和传递圆形布尔值，这会变得很难看。
将代码的相关部分重构为一个方法实际上是一种无操作：它重新排列了代码在文件中的布局方式，但对它的执行方式没有影响。

所有这些都让我相信，至少在这个问题的措辞中，标签是正确的解决方案，不需要重构。当然，在某些情况下标签使用不正确，应该重构掉。我只是不认为它应该被视为一些牢不可破的规则。

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 ); 在第二个循环结束时就可以了。如果代码要在逻辑上相同，则必须将其重写为如下所示：

for(int idx = 0; idx

score 1 · Accepted Answer

通过阅读您的代码。

我注意到您消除了 conditionAtVectorPosition 处的无效向量位置，然后您删除了 anotherConditionAtVector 处的无效行。
似乎在 anotherConditionAtVector 检查行是多余的，因为无论 idx 的值是什么， anotherConditionAtVector 仅取决于行索引（假设 anotherConditionAtVector 没有副作用）。

所以你可以这样做：

首先使用 conditionAtVectorPosition 获取有效位置（这些是有效列）。
然后使用 anotherConditionAtVector 获取有效行。
最后，使用有效的列和行来使用 conditionAtMatrixRowCol。

我希望这有帮助。

score 1 · Accepted Answer

@萨迪：

它们看起来都比原来的可读性差，因为它们涉及在代码机制而不是算法本身上花费更多的代码

将第二个循环外部化到算法之外不一定可读性较差。如果方法名选得好，可以提高可读性。

其中一些已损坏，或者在编辑之前就已损坏。最糟糕的是，人们不得不非常努力地思考如何编写没有标签的代码并且不破坏任何东西。

我有不同的观点：其中一些被破坏了，因为很难弄清楚原始算法的行为。

有些会带来两次运行相同测试的性能损失，这可能并不总是微不足道的。另一种方法是存储和传递圆形布尔值，这会变得很难看。

性能损失很小。但是我同意两次运行测试不是一个好的解决方案。

将代码的相关部分重构为一个方法实际上是一种无操作：它重新排列了代码在文件中的布局方式，但对它的执行方式没有影响。

我不明白这一点。是的，它不会改变行为，比如......重构？

当然，在某些情况下标签使用不正确，应该重构掉。我只是不认为它应该被视为一些牢不可破的规则。

我完全同意。但是正如你所指出的，我们中的一些人在重构这个例子时遇到了困难。即使最初的示例是可读的，也很难维护。

score 1 · Accepted Answer

@尼古拉斯

其中一些已损坏，或者在编辑之前就已损坏。最糟糕的是，人们不得不非常努力地思考如何编写没有标签的代码并且不破坏任何东西。

我有不同的观点：其中一些被破坏了，因为很难弄清楚原始算法的行为。

我意识到这是主观的，但我在阅读原始算法时没有任何问题。它比建议的替代品更短更清晰。

该线程中的所有重构都是使用其他语言功能模拟标签的行为 - 就好像您将代码移植到没有标签的语言一样。

score 1 · Accepted Answer

有些会带来两次运行相同测试的性能损失，这可能并不总是微不足道的。另一种方法是存储和传递圆形布尔值，这会变得很难看。
性能损失很小。但是我同意两次运行测试不是一个好的解决方案。

我相信问题是如何去除标签，而不是如何优化算法。在我看来，原始发帖人不知道如何使用没有标签的“继续”和“中断”关键字，但当然，我的假设可能是错误的。

在性能方面，该帖子没有提供有关其他功能实现的任何信息，因此据我所知，他们可能会通过 FTP 下载结果，因为它由编译器内联的简单计算组成。

话虽如此，在理论上进行两次相同的测试并不是最优的。

编辑：再想一想，这个例子实际上并不是对标签的可怕使用。我同意"goto is a no-no"，但不是因为这样的代码。这里使用标签实际上并不会显着影响代码的可读性。当然，它们不是必需的，可以很容易地省略，但不要仅仅因为“使用标签是不好的”而使用它们，在这种情况下并不是一个好的论据。毕竟，正如其他人已经评论的那样，删除标签并没有使代码更易于阅读。

score 1 · Accepted Answer

这个问题不是关于优化算法 - 但无论如何谢谢;-)

在我写它的时候，我认为标记为 continue 是一个可读的解决方案。

我问了一个关于Java 中标签的约定（标签是否全部大写）的问题。

基本上每个答案都告诉我“不要使用它们 - 总有更好的方法！重构！”。所以我发布了这个问题，要求提供更易读（因此更好？）的解决方案。

到目前为止，我并不完全相信迄今为止提出的替代方案。

请不要误会我的意思。标签在大多数时候都是邪恶的。

但就我而言，条件测试非常简单，算法取自数学论文，因此很可能在不久的将来不会改变。因此，我更喜欢一次显示所有相关部分，而不是滚动到另一个名为 checkMatrixAtRow(x) 的方法。

尤其是在更复杂的数学算法中，我发现很难找到“好的”函数名——但我想这是另一个问题

score 1 · Accepted Answer

我认为标记循环是如此罕见，以至于您可以选择适合您的任何标记方法 - 您在那里拥有的内容使您的意图与 continue 非常清楚。

在带头建议重构原始问题中的循环并且现在看到有问题的代码之后，我认为您在那里有一个非常易读的循环。

我想象的是一段非常不同的代码——把实际的例子放在上面，我可以看到它比我想象的要干净得多。

对于造成的误解，我深表歉意。

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

Gishu 有正确的想法：

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

我不太确定理解第一个继续。我会复制 Gishu 并写下类似的内容（如果有错误，请见谅）：

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