logo标签列表

C#: 并非所有的代码路径都返回值

c#compilation
3
我正在编写一个简单的WinForms应用程序，其中我允许用户在TreeView控件中拖动TreeNodes。 我执行的规则之一是，不允许用户将TreeNode拖入其自身的子节点中。 我以递归方式编写了以下函数来检查目标节点的父级关系。 在编译时，我收到此函数的并非所有代码路径都返回值的错误。据我所知，我在这个逻辑的每个可能的分支上都有一个返回语句...但我显然是错的。请问是否有人可以指出我的错误？
    private bool IsDestinationNodeAChildOfDraggingNode(TreeNode draggingNode, TreeNode destinationNode) {
        if (draggingNode.Nodes.Count == 0) 
            return false;
        else {
            if (draggingNode.Nodes.Contains(destinationNode)) 
                return true;
            else {
                foreach (TreeNode node in draggingNode.Nodes) 
                    return IsDestinationNodeAChildOfDraggingNode(node, destinationNode);
            }
        }
    }
6
大家都在纠正编译器错误，但没有注意到算法只探索了图的一条路径。这几乎肯定是错误的。 - Jherico
7个回答
12

它可能是指如果draggingNode.Nodes中没有任何项，那么它将跌出else，并在不返回任何内容的情况下退出函数。

也许可以这样做：

foreach (TreeNode node in draggingNode.Nodes) 
     return IsDestinationNodeAChildOfDraggingNode(node, destinationNode);

return false
3
代码检查器不知道...你对它理解算法表示赞赏。它只能理解代码路径（其中一个路径被Karim指出漏掉了）。 - Godeke
1
可能需要解释一下为什么会这样，因为人们可能会认为开头的计数检查就足够了...但在多线程和/或事件驱动环境中，不能保证这是正确的（尽管我不知道编译器是否真的考虑了这个问题，或者它只是挑剔/简单） - Oskar Duveborn
这个解决方案实际上并不起作用，因为如果目标超过2层深，则返回false... - λ Jonas Gorauskas
1
@Jonas，错误的，如果目标节点不是图中左侧某个节点的兄弟节点，则返回false。 - Jherico
如果draggingNode.Nodes没有任何项，则它的Count属性将等于零[...] 真正 - 如果（！）Count属性执行您期望的操作.....因此，在这种情况下，保守检查是有道理的。 - danielschemmel
显示剩余2条评论
8

实际上，我认为你的算法是错误的。如果你只想检查第一个值，那么为什么要使用foreach语句呢？此外，在返回之后的所有else都有点多余，会使代码更难理解。

我猜想你想做的事情更像是这样：

private bool IsDestinationNodeAChildOfDraggingNode(TreeNode draggingNode, TreeNode destinationNode) {
  // special case, no children
  if (draggingNode.Nodes.Count == 0) 
    return false;

  // check if the target is one of my children
  if (draggingNode.Nodes.Contains(destinationNode)) 
     return true;

  // recursively check each of my children to see if one of their descendants is the target
  foreach (TreeNode node in draggingNode.Nodes) 
    if (IsDestinationNodeAChildOfDraggingNode(node, destinationNode)) 
        return true;

  // didn't find anything
  return false;
}

有些人会坚持认为早期返回（early return）是一种恶习，这可能导致函数的以下版本：

private bool IsDestinationNodeAChildOfDraggingNode(TreeNode draggingNode, TreeNode destinationNode) {
  bool retVal = false;

  if (draggingNode.Nodes.Count != 0) {

    // check if the target is one of my children
    if (draggingNode.Nodes.Contains(destinationNode)) {
      retVal = true;
    } else {

      // recursively check each of my children to see if one of their descendants is the target
      foreach (TreeNode node in draggingNode.Nodes) 
        if (IsDestinationNodeAChildOfDraggingNode(node, destinationNode)) {
          retVal = true;
          break;
        }
    }
  }

  return retVal;
}
1
我的逻辑如下：
  1. 如果draggingNode没有节点，则destinationNode不能是其子节点之一。
  2. 否则，检查draggingNode的直接子节点是否包含destinationNode。
  3. 如果不是，则开始进入draggingNode的孙子节点，直到没有可能为止。
顺便说一句，您创建bool retVal = false;的替代方案有效，并产生了期望的效果。我认为早期返回版本更加简洁。为什么你说早期返回是邪恶的？ - λ Jonas Gorauskas
2
步骤1和2没问题，但第3步有问题，因为代码总是基于第一个子节点的子节点返回true或false，而不是所有孙子节点。如果您的第二个子节点的第一个子节点是目标节点，则您的代码仍将返回false。此外，我并没有说早期返回是邪恶的。我说有些人可能会这么说。我去过一些编码指南禁止它们的地方。我不知道理由。 - Jherico
1
关于“早期返回”，这是这个问题的主题：https://dev59.com/PXVD5IYBdhLWcg3wQZUg - Jherico
每当早期返回等于更简单、更易于理解的代码时，我很乐意晚餐吃意大利面。 - snarf
1
@Jherico - 感谢您指出早期返回问题的问题...那里有有趣的评论。我接受了Jherico的答案，因为他解决了编译器错误和我的算法缺陷。 - λ Jonas Gorauskas
2
编译器认为您需要在 foreach 循环后添加一个 return，因为可能会出现 draggingNode.Nodes 中没有节点的情况，因此循环实际上不会执行。
private bool IsDestinationNodeAChildOfDraggingNode(TreeNode draggingNode, TreeNode destinationNode) {
    if (draggingNode.Nodes.Count == 0) 
        return false;
    else {
        if (draggingNode.Nodes.Contains(destinationNode)) 
            return true;
        else {
            foreach (TreeNode node in draggingNode.Nodes) 
            {
                return IsDestinationNodeAChildOfDraggingNode(node, destinationNode);
            }
            return false; // <-- here for example
        }
    }
}

现在这可能不是您算法的正确位置，但编译器认为此处缺少一个 return。按照Jherico建议的方式重新构建代码将使情况更加清晰。

我实际上正在确定draggingNode.Nodes是否有任何节点，当我检查*if (draggingNode.Nodes.Count == 0)*时...所以当我进入foreach循环时，我已经知道我有要循环的节点了...这不正确吗？ - λ Jonas Gorauskas
@Jonas - 我理解你的观点，但在指定位置添加返回语句可以防止编译器报错。 - ChrisF
在您提出的地方添加return false语句是行不通的...当目标节点超过2个级别时，它会返回false... - λ Jonas Gorauskas
1
如果draggingNodes.Nodes中没有任何项，那么你将不会返回任何值。
0

在外层 Else 之外，必须返回某些内容。

例如：

private bool IsDestinationNodeAChildOfDraggingNode(TreeNode draggingNode, TreeNode destinationNode) {
if (draggingNode.Nodes.Count == 0) 
        return false;
else {
       if (draggingNode.Nodes.Contains(destinationNode)) 
            return true;
       else {
            foreach (TreeNode node in draggingNode.Nodes) 
            return IsDestinationNodeAChildOfDraggingNode(node, destinationNode);
            }
    }
//Return something here.
}
如果修复了foreach循环的问题，内部if/else的两个分支都会返回值，编译器就会满意了。 - Bevan
0

我的期望是foreach循环不能保证返回一个值，如果所有元素都没有返回任何内容，或者没有节点。在最后的if/else/foreach之后，添加一个默认的返回值。

0

我猜测 C# 编译器假设 foreach 循环正常终止，而不是跨越约三个控制层级返回。

Karim的答案比我的好多了。 - John R. Strohm
网页内容由stack overflow 提供, 点击上面的
可以查看英文原文，
原文链接