Я немного смущен тем, как лучше всего реорганизовать мой код во что-то более читаемое.
Рассмотрим этот фрагмент кода:
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, необходимых, потому что каждое вложенное, если оно зависит от предыдущих значений.
Теперь мне было интересно, как сделать мой код немного более чистым в этом отношении.
Некоторые параметры, о которых я подумал, будут:
ArgumentNullException s, после чего я поймаю их в конце и поставлю оператор return в мое предложение finally (или вне блока try/catch)goto:Большинство из этих параметров кажутся мне "грязными", поэтому мне было интересно, есть ли хороший способ очистить этот беспорядок, который я создал.
Я бы выбрал несколько операторов return. Это упрощает чтение и понимание кода.
Не используйте goto по понятным причинам.
Не используйте исключения, потому что проверка, которую вы делаете, не является исключительной, это то, что вы можете ожидать, поэтому вы должны просто учитывать это. Программирование на исключениях также является анти-шаблоном.
Рассмотрим инвертирование нулевых проверок на:
var foo = getfoo();
if (foo == null)
{
return;
}
var bar = getbar(foo);
if (bar == null)
{
return;
}
...etc
Вы можете цеплять выражения. Назначение возвращает назначенное значение, поэтому вы можете проверить его результат. Кроме того, вы можете использовать назначенную переменную в следующем выражении.
Как только выражение возвращает 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 )
{
..
}
goto.В таких функциях часто будут выделяться ресурсы, которые выделяются или изменения состояния, которые выполняются в середине, которые необходимо отменить, прежде чем функция выйдет.
Обычная проблема с решениями на основе возврата (например, rexcfnghk и Gerrie Schenck's) заключается в том, что вам нужно помнить об отмене этих изменений состояния перед каждым возвратом. Это приводит к дублированию кода и открывает двери для тонких ошибок, особенно в больших функциях. Не делайте этого.
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;
Сначала ваше предложение (возврат после каждого предложения 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 на нулевой вход, поэтому вы будете иметь
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
Сделайте это в старой школе:
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.
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.
Альтернативой является использование "поддельного" одиночного цикла для управления потоком программы. Я не могу сказать, что рекомендую его, но это определенно лучше выглядит и более читабельным, чем стрелка.
Добавление "stage", "phase" или sth, как эта переменная, может упростить отладку и/или обработку ошибок.
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 * медленны и что в отладчике трудно сказать, какой метод вернул нулевое значение.
Ответ Рекса Керра действительно очень приятный.
Если вы можете изменить код, хотя ответ Jens Schauder, вероятно, лучше (шаблон Null Object)
Если вы можете сделать пример более конкретным, вы, возможно, получите еще больше ответов
Например, в зависимости от "местоположения" методов вы можете иметь что-то вроде:
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;
}
}
Странно, никто не упоминает цепочку методов.
Если вы создадите один раз класс цепочки методов
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
Вы можете использовать его везде, чтобы составить любое количество функций в последовательность выполнения, чтобы создать результат или ничего (нуль)
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.
Ваш пример может быть недостаточно, чтобы подтолкнуть меня к краю, и несколько возвратов лучше, если ваш метод достаточно прост. Но этот шаблон может стать довольно обширным, и вам часто нужен код очистки в конце. Хотя при использовании большинства других ответов здесь, если можно, часто единственное разборчивое решение - использовать goto.
(Когда вы это сделаете, обязательно поместите все ссылки на метку внутри одного блока, чтобы любой, кто смотрит код, знал как goto, так и переменные ограничивались этой частью кода.)
В 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, что заставляет меня думать, что другие люди заметили, что в этой ситуации они вам понадобятся.
Исключение будет работать и для меня, за исключением попытки try/catch, которую вы нажимаете на вызывающий код. Кроме того, С# помещает трассировку стека в бросок, что замедлит ваш код вниз, особенно если он обычно запускается при первой проверке.