如何防止箭头反模式

我对如何最好地将我的代码重构成更可读的东西有点困惑。

考虑这段代码:

var foo = getfoo(); if(foo!=null) { var bar = getbar(foo); if(bar!=null) { var moo = getmoo(bar); if(moo!=null) { var cow = getcow(moo); ... } } } return; 

正如你所看到的那样,嵌套if块是需要的,因为每个嵌套的if依赖于前面的值。

现在我想知道如何让我的代码在这方面更清洁。

我想到的一些select是:

  • 在每个if子句之后返回,这意味着我将有多个地方离开我的方法
  • 抛出ArgumentNullException s,之后我会赶上他们,并将return语句放在我的finally子句中(或在try / catch块之外)
  • 使用标签工作并goto:

这些选项中的大部分对我来说似乎有些“肮脏”,所以我想知道是否有一个很好的方法来清理我创build的这个混乱。

我会去多个return语句。 这使得代码易于阅读和理解。

明显的原因不要使用goto

不要使用例外,因为您所做的检查不是特例,这是您可以预期的,所以您应该考虑这一点。 针对exception进行编程也是一种反模式。

考虑反转空检查:

 var foo = getfoo(); if (foo == null) { return; } var bar = getbar(foo); if (bar == null) { return; } ...etc 

你可以链expression式。 分配会返回分配的值,因此您可以检查其结果。 另外,您可以在下一个expression式中使用赋值的variables。

只要expression式返回false,其他的不再执行,因为整个expression式已经返回false(因为and操作)。

所以,这样的事情应该工作:

 Foo foo; Bar bar; Moo moo; Cow cow; if( (foo = getfoo()) != null && (bar = getbar(foo)) != null && (moo = getmoo(bar)) != null && (cow = getcow(moo)) != null ) { .. } 

这是使用goto完全可以接受(如果不是所希望的)的几个场景之一。

在像这样的函数中,经常会有分配的资源或中途进行的状态更改,在函数退出之前需要将其撤销。

基于回报的解决scheme(例如,rexcfnghk和Gerrie Schenck)通常会遇到的问题是, 每次返回之前需要记住撤消这些状态更改 。 这会导致代码重复,并为微妙的错误打开大门,特别是在大型函数中。 不要这样做。


CERT 实际上推荐了基于goto的结构化方法。

特别要注意他们的代码,取自Linux内核的kernel/fork.c中的copy_process 。 这个概念的简化版本如下:

  if (!modify_state1(true)) goto cleanup_none; if (!modify_state2(true)) goto cleanup_state1; if (!modify_state3(true)) goto cleanup_state2; // ... cleanup_state3: modify_state3(false); cleanup_state2: modify_state2(false); cleanup_state1: modify_state1(false); cleanup_none: return; 

实质上,这只是“箭头”代码的更可读的版本,不使用不必要的缩进或重复的代码。 这个概念可以很容易地扩展到任何最适合您的情况。


作为最后一点,特别是关于CERT的第一个兼容示例,我只想补充一点,只要有可能,devise代码更简单一些,以便一次处理清理。 这样,你可以写这样的代码:

  FILE *f1 = null; FILE *f2 = null; void *mem = null; if ((f1 = fopen(FILE1, "r")) == null) goto cleanup; if ((f2 = fopen(FILE2, "r")) == null) goto cleanup; if ((mem = malloc(OBJSIZE)) == null) goto cleanup; // ... cleanup: free(mem); // These functions gracefully exit given null input close(f2); close(f1); return; 

首先你的build议(每条if条款之后返回)是一个很好的出路:

  // Contract (first check all the input) var foo = getfoo(); if (Object.ReferenceEquals(null, foo)) return; // <- Or throw exception, put assert etc. var bar = getbar(foo); if (Object.ReferenceEquals(null, bar)) return; // <- Or throw exception, put assert etc. var moo = getmoo(bar); if (Object.ReferenceEquals(null, moo)) return; // <- Or throw exception, put assert etc. // Routine: all instances (foo, bar, moo) are correct (not null) and we can work with them ... 

第二种可能性(在你的情况下)是稍微修改你的getbar()以及getmoo()函数,使得它们在nullinput时返回null,所以你将有

  var foo = getfoo(); var bar = getbar(foo); // return null if foo is null var moo = getmoo(bar); // return null if bar is null if ((foo == null) || (bar == null) || (moo == null)) return; // <- Or throw exception, put assert(s) etc. // Routine: all instances (foo, bar, moo) are correct (not null) ... 

第三种可能性是在复杂的情况下可以使用Null Object Desing Patteren

http://en.wikipedia.org/wiki/Null_Object_pattern

做老派:

 var foo; var bar; var moo; var cow; var failed = false; failed = failed || (foo = getfoo()) == null; failed = failed || (bar = getbar(foo)) == null; failed = failed || (moo = getmoo(bar)) == null; failed = failed || (cow = getcow(moo)) == null; 

更清楚 – 没有箭头 – 并可以永久延伸。

请不要走到Dark Side ,使用gotoreturn

 var foo = getFoo(); var bar = (foo == null) ? null : getBar(foo); var moo = (bar == null) ? null : getMoo(bar); var cow = (moo == null) ? null : getCow(moo); if (cow != null) { ... } 

如果你可以改变你正在调用的东西,你可以改变它永远不会返回null,而是一个NULL-Object 。

这将允许你完全失去所有的ifs。

另一种方法是使用“假”单循环来控制程序stream程。 我不能说我会推荐它,但它比箭头更好看,更可读。

添加一个“阶段”,“阶段”或像这个variables可能会简化debugging和/或error handling。

 int stage = 0; do { // for break only, possibly with no indent var foo = getfoo(); if(foo==null) break; stage = 1; var bar = getbar(foo); if(bar==null) break; stage = 2; var moo = getmoo(bar); if(moo==null) break; stage = 3; var cow = getcow(moo); return 0; // end of non-erroreous program flow } while (0); // make sure to leave an appropriate comment about the "fake" while // free resources if necessary // leave an error message ERR("error during stage %d", stage); //return a proper error (based on stage?) return ERROR; 
 try { if (getcow(getmoo(getbar(getfoo()))) == null) { throw new NullPointerException(); } catch(NullPointerException ex) { return; //or whatever you want to do when something is null } //... rest of the method 

这保持了方法的主要逻辑整齐,只有一个例外的回报。 它的缺点是,如果get *方法很慢,它可能会很慢,并且在debugging器中很难告诉哪个方法返回null值。

雷克斯·克尔的答案确实非常好。
如果你可以更改代码,Jens Schauder的答案可能会更好(空对象模式)

如果你可以让这个例子更具体,你可能会得到更多的答案
例如,根据方法的“位置”,您可以使用以下方法:

 namespace ConsoleApplication8 { using MyLibrary; using static MyLibrary.MyHelpers; class Foo { } class Bar { } class Moo { } class Cow { } internal class Program { private static void Main(string[] args) { var cow = getfoo()?.getbar()?.getmoo()?.getcow(); } } } namespace MyLibrary { using ConsoleApplication8; static class MyExtensions { public static Cow getcow(this Moo moo) => null; public static Moo getmoo(this Bar bar) => null; public static Bar getbar(this Foo foo) => null; } static class MyHelpers { public static Foo getfoo() => null; } } 

奇怪的是,没有人提到方法链。

如果你创build一个方法链接类

 Public Class Chainer(Of R) Public ReadOnly Result As R Private Sub New(Result As R) Me.Result = Result End Sub Public Shared Function Create() As Chainer(Of R) Return New Chainer(Of R)(Nothing) End Function Public Function Chain(Of S)(Method As Func(Of S)) As Chainer(Of S) Return New Chainer(Of S)(Method()) End Function Public Function Chain(Of S)(Method As Func(Of R, S)) As Chainer(Of S) Return New Chainer(Of S)(If(Result Is Nothing, Nothing, Method(Result))) End Function End Class 

您可以在任何地方使用它来将任意数量的函数组合成一个执行序列来产生一个Result或Nothing(Null)

 Dim Cow = Chainer(Of Object).Create. Chain(Function() GetFoo()). Chain(Function(Foo) GetBar(Foo)). Chain(Function(Bar) GetMoo(Bar)). Chain(Function(Moo) GetCow(Moo)). Result 

这是我使用goto的一种情况

你的例子可能不足以推动我的边缘,如果你的方法很简单,多个返回更好。 但是这种模式可以得到相当广泛的,最后你经常需要一些清理代码。 如果可以的话,尽可能使用其他答案,但通常唯一可行的解​​决scheme是使用goto

(当你这样做的时候,一定要把所有对标签的引用都放到一个块中,这样任何查看代码的人都会知道goto的variables和variables被限制在那部分代码中。)

在Javascript和Java中,你可以这样做:

 bigIf: { if (!something) break bigIf; if (!somethingelse) break bigIf; if (!otherthing) break bigIf; // Conditionally do something... } // Always do something else... return; 

Javascript和Java没有任何关系,这让我相信其他人已经注意到在这种情况下,你确实需要它们。

一个例外也适用于我,除了你在调用代码上强加的try / catch混乱。 另外 ,C#在这个throw中join了一个堆栈跟踪,这会减慢你的代码的速度,特别是当它通常在第一次检查时踢出来的时候。