52

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

考虑这段代码:

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 都依赖于先前的值。

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

我认为自己的一些选择是:

  • 在每个 if 子句之后返回,这意味着我将在多个地方留下我的方法
  • throw ArgumentNullExceptions,之后我会在最后捕获它们并将 return 语句放在我的 finally 子句中(或在 try/catch 块之外)
  • 使用标签和goto:

这些选项中的大多数对我来说似乎有点“脏”,所以我想知道是否有一种好方法可以清理我创建的这个烂摊子。

4

13 回答 13

47

我会选择多个return陈述。这使得代码易于阅读和理解。

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

不要使用异常,因为您所做的检查并非异常,这是您可以期待的,因此您应该将其考虑在内。针对异常进行编程也是一种反模式。

于 2013-07-23T07:35:49.433 回答
44

考虑将空检查反转为:

var foo = getfoo();
if (foo == null)
{
    return;
}
var bar = getbar(foo);
if (bar == null)
{
    return;
}
...etc
于 2013-07-23T07:34:08.370 回答
27

您可以链接表达式。分配返回分配的值,因此您可以检查其结果。此外,您可以在下一个表达式中使用分配的变量。

一旦一个表达式返回 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 )
{
  ..
}
于 2013-07-23T07:40:30.213 回答
25

这是完全可以接受(如果不希望)使用goto.

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

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


CERT实际上推荐了一种基于goto.

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

    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;
于 2013-07-23T16:36:29.417 回答
16

首先,您的建议(在每个 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

于 2013-07-23T07:51:58.817 回答
11

做老派:

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

于 2013-07-23T14:20:28.220 回答
5
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) {
  ...
}
于 2013-07-23T16:20:40.047 回答
3

如果您可以更改您正在调用的内容,则可以将其更改为永远不会返回 null,而是返回NULL-Object

这将使您完全失去所有的如果。

于 2013-07-23T11:42:15.660 回答
1

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

添加“阶段”、“阶段”或类似的变量可以简化调试和/或错误处理。

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;
于 2013-07-24T10:04:20.507 回答
1
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* 方法很慢,它可能会很慢,并且很难在调试器中判断哪个方法返回了 null 值。

于 2013-07-24T12:21:57.993 回答
1

Rex Kerr 的回答确实很好。
如果您可以更改代码,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;
    }
}
于 2017-04-30T22:13:57.117 回答
0

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

如果您创建一次方法链接类

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

您可以在任何地方使用它来将任意数量的函数组合成一个执行序列以产生一个结果或一个空(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
于 2015-10-26T21:42:45.757 回答
-3

这是我使用 goto 的一种情况

您的示例可能不足以将我推向边缘,如果您的方法足够简单,则多次返回会更好。但是这种模式可能会变得相当广泛,并且您通常需要最后一些清理代码。如果可以的话,虽然在这里使用大多数其他答案,但通常唯一清晰的解决方案是使用goto's.

(当你这样做时,请确保将所有对标签的引用放在一个块内,这样任何查看代码的人都知道goto's 和变量都被限制在代码的那一部分。)

在 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 没有goto's,这让我相信其他人已经注意到在这种情况下你确实需要它们。

一个例外对我也有用,除了你在调用代码上强加的 try/catch 混乱。 此外,C# 会在 throw 时放入堆栈跟踪,这会减慢您的代码速度,特别是如果它通常在第一次检查时退出。

于 2013-07-23T14:40:23.373 回答