如何防止箭头反模式

Question

我对如何最好地将我的代码重构为更具可读性的东西感到困惑.

考虑一下这段代码:

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子句后返回,这意味着我有多个地方可以保留我的方法
• 抛出ArgumentNullExceptions,之后我将它们捕获到最后并将return语句放在我的finally子句中(或者在try/catch块之外)
• 使用标签和 goto:

大多数这些选项对我来说似乎有点"脏",所以我想知道是否有一种很好的方法来清理我创建的这个混乱.

Answer 1

我会去做多个return陈述.这使代码易于阅读和理解.

不要goto出于明显的原因使用.

不要使用异常,因为您正在进行的检查并不例外,这是您可以预期的,因此您应该考虑到这一点.针对异常的编程也是一种反模式.

Answer 2

考虑将空检查反转为:

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

Answer 3

你可以链接表达式.赋值返回指定的值,因此您可以检查其结果.此外,您可以在下一个表达式中使用指定的变量.

一旦表达式返回false,其他表达式就不再执行,因为整个表达式将返回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 )
{
  ..
}

Answer 4

这是少数情况之一,使用它是完全可以接受的(如果不是可取的话)goto.

在这样的函数中,通常会有中间分配的资源或状态更改,需要在函数退出之前撤消.

基于回归的解决方案(例如,rexcfnghk和Gerrie Schenck)的常见问题是您需要记住在每次返回之前撤消这些状态更改.这会导致代码重复,并打开微妙错误的大门,尤其是在较大的功能中. 不要这样做.

CERT 实际上建议采用基于的结构方法goto.

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

    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的第一个兼容示例,我只想补充一点,只要有可能,设计代码就更简单,以便可以一次性处理清理.这样,您可以编写如下代码:

    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;

Answer 5

首先你的建议(每个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()函数,使得它们在null输入时返回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

Answer 6

做老派:

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使用goto或return.

Answer 7

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) {
  ...
}