Стандарты кодирования / Лучшие практики кодирования в C ++

Question

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

// Код 1

bool MyApplication::ReportGenerator::GenerateReport(){
    bool retval = false;
    do{
        if (!isAdmin()){
            break;
        }
        if (!isConditionOne()){
            break;
        }
        if (!isConditionTwo()){
            break;
        }
        if (!isConditionThree()){
            break;
        }

        retval = generateReport();
    } while(0);
    return retval;
}

// Кодекса2

bool MyApplication::ReportGenerator::GenerateReport(){
    if (!isAdmin()  || !isConditionOne() || !isConditionTwo() || !isConditionThree()){
        return false;
    }
    return generateReport();
}

Чистый код Роберта С. Мартина - хорошая книга, которая имеет дело с этим. Но, я думаю, эта книга склонна к Java.

UPDATE:

1. Я специально использовал do {} while (0); цикл, поскольку я не хотел использовать goto.

2. Я хотел избавиться от стольких операторов if и break, поэтому я предложил код 2.

То, что я вижу из ответов, представляет собой смешанный набор ответов как для Code1, так и для Code2. Есть люди, которые также предпочитают goto по сравнению с Кодексом 1 (который я считал лучше).

Answer 1

Мне не очень нравится использовать цикл do / while таким образом. Еще один способ сделать это состоит в том, чтобы разбить ваше условие в Code2 на отдельные проверки if. Их иногда называют «пунктами охраны».

bool MyApplication::ReportGenerator::GenerateReport()
{
    if (!isAdmin())
        return false;

    if (!isConditionOne())
        return false;

    // etc.

    return generateReport();

}

Answer 2

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

bool MyApplication::ReportGenerator::GenerateReport()
{
    if(isAdmin() && isConditionOne() && isConditionTwo() && isConditionThree())
    {
        return generateReport();
    }

    return false;
}

Там сказано все в одном чистом месте. «Если все эти условия соблюдены, то сделайте это. В противном случае не выполняйте».

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

Answer 3

bool MyApplication::ReportGenerator::GenerateReport()
{
   return isAdmin()  
      && isConditionOne() 
      && isConditionTwo()
      && isConditionThree()
      && generateReport();    // Everything's ok.
}

Answer 4

bool MyApplication::ReportGenerator::GenerateReport(){
    if ( ! isAdmin         () ) return false ;
    if ( ! isConditionOne  () ) return false ;
    if ( ! isConditionTwo  () ) return false ;
    if ( ! isConditionThree() ) return false ;
    return generateReport() ;
}

Answer 5

Это действительно зависит от будущих ожиданий кода. Код1 выше подразумевает, что может быть добавлена ​​дополнительная логика для каждого из условий; Приведенный выше код2 скорее подразумевает, что существует рациональная группировка условных выражений. Код1 может быть более уместным, если вы планируете добавить логику для условий позднее; если вы этого не сделаете, Code2, вероятно, более разумно из-за краткости и подразумеваемой группировки.

Answer 6

Я предпочитаю модификацию образца 2:

bool MyApplication::ReportGenerator::GenerateReport()
{
    bool returnValue = false;
    if (isAdmin() &&
        isConditionOne() &&
        isConditionTwo() &&
        isConditionThree())
    { 
        returnValue = generateReport();
    } 
    return returnValue;
}

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

Answer 7

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

Я бы сделал это:

bool MyApplication::ReportGenerator::GenerateReport(){
    if (isAdmin()  && isConditionOne() && isConditionTwo() && isConditionThree()){
         return generateReport();
    } else {
        return false;
    }

}

Потому что:

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

Answer 8

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

Использование:

   if (condition_1) return false;
   if (condition_2) return false;
   ...

будет лучше.

Кроме того, что мне не нравится в коде 1, так это то, что он пытается замаскировать gotos, используя while и breaks (которые являются gotos). Тогда я предпочел бы использовать непосредственно goto, по крайней мере, было бы легче увидеть, где находится точка приземления.

Код 2 может быть отформатирован так, чтобы он выглядел красиво, я думаю:

 bool MyApplication::ReportGenerator::GenerateReport(){
     if (!isAdmin()        || !isConditionOne() ||
         !isConditionTwo() || !isConditionThree()) {
        return false;
     }
     return generateReport();
 }

Или что-то подобное.

Answer 9

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

bool isReportable(anyParametersNeeded){
    //stuffYouWantToCheck
}

bool MyApplication::ReportGenerator::GenerateReport(){
    if (isReportable(anyParametersNeeded)){
        return generateReport();
    }
    return false;
}

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

Answer 10

Относительно примера 1: Если вы хотите перейти на goto, почему бы вам не написать goto ??? Это намного понятнее, чем ваше предложение. И учитывая, что goto следует использовать редко, я голосую за № 2.