если / еще, хороший дизайн - PullRequest
18 голосов
/ 03 ноября 2010

Является ли приемлемым / хорошим стиль для упрощения этой функции:

bool TryDo(Class1 obj, SomeEnum type)
{
    if (obj.CanDo(type))
    {
        return Do(obj);
    }
    else
    {
        return false;
    }
}

как:

bool TryDo(Class1 obj, SomeEnum type)
{
    return obj.CanDo(type) && Do(obj);
}

Вторая версия короче, но, возможно, менее интуитивно понятна.

Ответы [ 20 ]

64 голосов
/ 03 ноября 2010

Что я хотел бы кодировать:

return obj.CanDo(type) ? Do(obj) : false;
24 голосов
/ 03 ноября 2010

Версия с скобками:

bool TryDo(Class1 obj, SomeEnum type)
{
    if (obj.CanDo(type))
    {
        return Do(obj);
    }

    return false;
}

Или версия без скобок (в комментариях к ответу идет спор об этом):

bool TryDo(Class1 obj, SomeEnum type)
{
    /*
     * If you want use this syntax of
     * "if", this doing this on self
     * responsibility, and i don't want
     * get down votes for this syntax,
     * because if I remove this from my
     * answer, i get down votes because many
     * peoples think brackets i wrong.
     * See comments for more information.
     */
    if (obj.CanDo(type))
        return Do(obj);

    return false;
}

Ваш первый пример кода лучше, но я думаю, что моя версия еще лучше.

Ваша вторая версия плохо читается и затрудняет поддержку кода , это плохо.

18 голосов
/ 03 ноября 2010

else бесполезен, а &&, хотя и очевиден, не так удобочитаем, как чистый текст

Я предпочитаю следующее:

bool TryDo(Class1 obj, SomeEnum type)
{
    if (obj.CanDo(type))
    {
        return Do(obj);
    }   
    return false;
}
13 голосов
/ 03 ноября 2010

Да.

Особенно с именами, похожими на выбранные вами имена, т.е. CanDoSomething и DoSomething. абсолютно ясно для любого компетентного программиста, что делает второй код: « тогда и только тогда, когда условие выполнено, что-то сделать и вернуть результат ».«Если и только если» - это основной смысл короткозамкнутого оператора &&.

Первый код является сложным и излишне длинным, не предоставляя больше информации, чем второй код.

Но в целом, эти два условия могут не образовывать такие тесные отношения (как в CanDo и Do), и может быть лучше разделить их логически, потому что помещение их в одно и то же условие может не иметь интуитивного смысла.

Многие люди здесь утверждают, что первая версия «намного понятнее».Я бы очень хотел услышать их аргументы.Я не могу думать ни о каком.

С другой стороны, есть этот тесно связанный (хотя и не совсем такой же) код:

if (condition)
    return true;
else
    return false;

это должно всегда преобразуется в это:

return condition;

Без исключения .Это краткое и еще более удобочитаемое для тех, кто владеет языком.

12 голосов
/ 03 ноября 2010

Сокращенная версия скрывает тот факт, что Do что-то делает.Похоже, вы просто делаете сравнение и возвращаете результат, но на самом деле вы делаете сравнение и выполняете действие, и не очевидно, что у кода есть этот «побочный эффект».

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

7 голосов
/ 03 ноября 2010

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

bool TryDo(Class1 obj, SomeEnum type) {
  return obj.CanDo(type) ? Do(obj) : false;
}
6 голосов
/ 03 ноября 2010

Мне не нравится этот дизайн, и, возможно, не по очевидной причине. Что меня беспокоит, так это вернуть Do (obj); Для меня не имеет смысла для функции Do иметь тип возврата bool. Является ли это заменой свойства, вызывающего ошибки? Скорее всего, эта функция должна быть void или возвращать сложный объект. Сценарий просто не должен придумывать. Также, если bool каким-то образом имеет смысл сейчас, он может легко перестать иметь смысл в будущем. С вашим изменением кода потребуется больше повторного факторинга для исправления

6 голосов
/ 03 ноября 2010

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

5 голосов
/ 03 ноября 2010

Ни один, потому что, когда TryDo возвращает False, вы не можете определить, было ли это потому, что «Не могу сделать» или «Вернуть не удалось».

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

Если результат не имеет смысла, намерение будет более ясным с

void TryDo(Class1 obj, SomeEnum type)
{
    if (obj.CanDo(type))
        Do(obj);
    return;
}

Если результат имеет значение, тогда должен быть способ различать два «ложных» результата. И.Е. Что значит «если (! TryDo (x))» означает ?

Редактировать: Другими словами, код ОП гласит, что «Я не умею плавать» - это то же самое, что «Я пытался плавать и утонул»

5 голосов
/ 03 ноября 2010

В этом случае, я бы выбрал первый вариант - он намного более читабелен, и смысл кода намного яснее.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...