Краткий проверку на null против читабельности


У меня есть случай, когда мне нужно вызвать пару методов, проверяя, если они возвращают значение null, и вернуть какое-то имущество из результата второго, когда не возвращает null.

Я придумал два способа сделать это - один "подробный" и "лаконичный". Для целей настоящего вопрос, я просто позвоню о "динамических", поскольку меня не интересуют эти подробности здесь.

private dynamic VerboseVersion()
{
     var a = CallMethodA();
     if ( a == null ) 
     { 
         return null; 
     }

     var b = CallMethodB( a );
     if ( b == null ) 
     { 
         return null; 
     }

     return b.SomeProperty;
}

private dynamic ConciseVersion()
{
    dynamic a;
    dynamic b;

    if ( ( a = CallMethodA() ) == null ||
         ( b = CallMethodB( a ) ) == null )
    {
        return null;
    }

    return b.SomeProperty;
}

Я первый, потому что умысла на каждом шагу очень очевидным, но я нахожу это несколько многословный, мне нравится второй, потому что цель функция кажется более очевидным и менее подробным, но это может быть немного сложнее, чтобы следовать в первую очередь.

Мысли?

Обновление: исправлена опечатка и исправил форматирование "подробный" вариант, что я бы на самом деле использовать в совершенных кодов.



585
5
c#
задан 2 августа 2011 в 07:08 Источник Поделиться
Комментарии
10 ответов

Я получаю огромное удовольствие от того, что ваш "сжатая" версия больше, чем ваш "подробный" вариант!

Если верить одной из версий, чтобы быть более понятным и других более непонятных, я бы спорить будет с читабельностью над мракобесием. Одну непонятную деталь заключается в том, что вы полагаетесь на людей понимания

(a = CallMethodA()) == null

Некоторые люди могут подумать, что это ошибка, не понимая выражения слева на самом деле имеет значение.

Если вы хотите использовать меньше строк кода, Вы могли бы пойти с чем-то подобным

var a = CallMethodA();
var b = (a == null) ? null : CallMethodB(a);
return (b == null) ? null : b.SomeProperty;

Что не совсем, чем ваш оригинал, если цепь/еще, но немного более сжат и имеет только один оператор return.

8
ответ дан 2 августа 2011 в 08:08 Источник Поделиться

Я всегда предпочту многословный один. Понятно, читаемо и самое главное это не просто понять его. Я никогда не пытаюсь сделать код короче, на самом деле я считаю, что даже если код больше общая стоимость может быть ниже.

Потому что это недоразумение или ошибка из-за неоднозначности всегда обходится намного больше, чем набрав несколько строк.

3
ответ дан 2 августа 2011 в 09:08 Источник Поделиться

Я тоже согласна с @oberfreak, что я чувствую, что в вашем примере вы используете null в качестве значения ошибки для управления ошибками и сделать некоторые защиты, рассматривали ли вы использование исключений?

Во многих случаях, чтобы избежать uncatched исключений и try-catch блоки многие из нас использовать null в качестве значения ошибки и, вероятно, использовать другой вид приблизительно будет хорошей альтернативой.

Также, другой вариант, я бы предложил называть CallMethodA внутри CallMethodB и контролировать возвращаемое значение CallMethodA

2
ответ дан 5 февраля 2014 в 12:02 Источник Поделиться

Я хотел изменить CallMethodB возвращать null, если он передается null, а затем использовать следующие:

private dynamic VerboseVersion()
{
var b = CallMethodB( CallMethodA() );
if ( b == null )
{
return null;
}
else
{
return b.SomeProperty;
}
}

Я предпочитаю, чтобы избежать присвоения в условиях.

1
ответ дан 2 августа 2011 в 08:08 Источник Поделиться

Эти две версии не имеют ту же семантику. Первая версия гарантирует CallMethodB получает ненулевой параметр "а", а второй-нет. Я не знаю, что CallMethodB требует, но вы могли бы сделать исключение нулевого указателя в Си. Я лично предпочитаю первый вариант, ясным и точным, я не чувствую тебя терять читабельность. Если CallMethodB принимает нулевые значения, тогда вы могли бы упростить код просто:

b = CallMethodB(CallMethodA());

Поправочка: как сказал Энтони, С "||" в конце вообще будет короткое замыкание оценки, когда значение null, так что нет никакой смысловой разницы в этих двух версия. Я неправильно понял исходный пост. Извините за мой прыжок в GNU заключение. Хотя, я все же предпочитаю версию 1, он чувствует себя так меньше путаницы.

0
ответ дан 2 августа 2011 в 08:08 Источник Поделиться

Я полностью согласен с Озкан Серкан.

Несмотря на то, что ConciseVersion выглядит намного "умнее" я бы не стал жертвовать redability и будут голосовать за VerboseVersion. Это то, что большинство разработчиков привыкли. Более того, я бы поставил return в отдельной строке, так что бы было понятно, что если что-то является нулем, то нуль должен быть возвращен

0
ответ дан 3 августа 2011 в 06:08 Источник Поделиться

Другой вариант-создать метод расширения, как:

    public static class GeneralExtensions
{
public static dynamic GetNext<T>(this T objectToProcess,
Func<T, dynamic> getNextCallback)
where T : class
{
if (objectToProcess != null)
{
return getNextCallback(objectToProcess);
}
else
{
return null;
}
}
}

Тогда ваш код будет:

return CallMethodA()
.GetNext(a => CallMethodB(a)
.GetNext(b => b.SomeProperty));

0
ответ дан 3 августа 2011 в 02:08 Источник Поделиться

Я думаю, что сжатая версия более и более запутанной, и подчеркивает неправильно возврат (при условии, что значение SomeProperty то, что реально процентов).

Для многословный, мне не нравится, возвращаясь тем же значением более чем один раз, даже если это постоянно - так вариация на тему многословен:

private dynamic MyVerboseVersion() 
{
var a = CallMethodA();
if (a != null) {
var b = CallMethodB(a);
if (b != null) {
return b.SomeProperty;
}
}

return null;
}

Я не очень люблю растущей стрелкой или осенью через другое, но это, как я часто решить этот шаблон.

0
ответ дан 3 августа 2011 в 07:08 Источник Поделиться

Если у меня есть возможность изменить методы, и ожидаем, что в результате метод возвращает значение null, я буду переписывать и следуйте TryXXX шаблон.

private dynamic TryVersion()
{
dynamic a;
dynamic b;

if (TryCallMethodA(out a)) {
if (TryCallMethodB( a, out b )){
return b.SomeProperty;
}
}

return null;
}

Если вы действительно хотите, чтобы сократить это, вы можете:

if (TryCallMethodA(out a) && TryCallMethodB( a, out b )){
return b.SomeProperty;
}

Примечание: второй фрагмент-это непроверенных, но я думаю, что он будет работать как левой TryCall должны быть решены, чтобы перейти на правильное TryCall.

0
ответ дан 4 августа 2011 в 07:08 Источник Поделиться

Почему методы CallMethodA и CallMethodB возвращать null? Разве не было бы лучше, чтобы бросить исключение, что вы можете сигнал, что что-то пошло ужасно неправильно?

На мой взгляд и возвращает значение NULL, это всегда плохо для разработчиков, которые используют ваш код, потому что они не знают, что пошло не так и что изменить.

В вашем примере вы ожидаете CallMethodB для возвращения объекта. Так что если никто не бросает исключение, просто все работало нормально, использовать возвращаемое значение, если это-нуль, пусть исключение NullReferenceException возникает и нравится писать код хорошо.

Удаление проверки будет увеличить читаемость и reusablility кода!

SampleObject CallMethodA(){
if(Some())
return new A();
throw new SomeThingTerribleException(Can't create SampleObject due to lack of Some");
}

SampleObject CallMethodB(SampleObject obj){
if(obj.IsSomethingSet())
return new B(obj);
throw new SomeThingTerribleException("Can't create SampleObject due to lack of Some");
}
void Main(){
var a = CallMethodA();
var b = CallMethodB( a );
return b.SomeProperty;
}

Как видите, нет нуль, никаких грязных нуль-проверяет только четких исключений для пользователей ваших методов.

0
ответ дан 30 сентября 2011 в 02:09 Источник Поделиться