Это злоупотребление попыткой / наконец? - PullRequest
31 голосов
/ 09 февраля 2010

Учитывая, что допустимы множественные операторы возврата (я не согласен, но давайте отвлечемся ), я ищу более приемлемый способ достижения следующего поведения:

Опция A : многократный возврат, повторный кодовый блок

public bool myMethod() {
    /* ... code ... */

    if(thisCondition) {
        /* ... code that must run at end of method ... */
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        /* ... the SAME code that must run at end of method ... */
        return false;
    }

    /* ... even more code ... */

    /* ... the SAME CODE AGAIN that must run at end of method ... */
    return lastCondition;
}

Мне грустно видеть, что один и тот же (маленький) блок кода повторяется три раза каждый раз, когда метод возвращается. Кроме того, я хотел бы уточнить, что два return false утверждения, приведенные выше, безусловно, можно описать как возвращающие промежуточный метод ... они абсолютно не являются «защитными утверждениями».

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

Опция B : множественные возвраты, блок try / finally (без блоков / исключений catch)

public bool myMethod() {
    try {
        /* ... code ... */

        if(thisCondition) {
            return false;
        }

        /* ... more code ... */

        if(thatCondition) {
            return false;
        }

        /* ... even more code ... */

        return lastCondition;
    } finally {
        /* ... code that must run at end of method ... */
    }
}

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

Опция C : одинарный возврат, условные блоки

public bool myMethod() {
    /* ... code ... */

    if(!thisCondition) {
        /* ... more code ... */
    }

    if(!thisCondition && !thatCondition) {
        /* ... even more code ... */
    }

    /* ... code that must run at end of method ... */
    return summaryCondition;
}

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

Ответы [ 11 ]

29 голосов
/ 09 февраля 2010

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

Если его не нужно запускать в случае исключения (т. Е. Это необходимо только для «нормальных» возвратов), то использование finally приведет к злоупотреблению функцией.

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

Когда этот код оказывается слишком сложным (и это очень реальная возможность), тогда пришло время провести рефакторинг метода, извлекая один или несколько методов.

Самым простым рефакторингом будет что-то вроде этого:

public boolean  myMethod() {
    boolean result = myExtractedMethod();
    /* ... code that must run at end of method ... */
    return result;
}

protected boolean myExtractedMethod() {
    /* ... code ... */

    if(thisCondition) {
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        return false;
    }

    /* ... even more code ... */
    return lastCondition;
}
28 голосов
/ 09 февраля 2010

Исключения должны быть исключительными, поэтому мне не нравится опция B, если вокруг нет других исключений (Примечание для downvoters - я не говорю, что наличие finally неправильно, просто я предпочитаю не иметь его, если нет исключений - если у вас есть причины, пожалуйста, прокомментируйте)

Если код всегда нужен, как насчет рефакторинга на 2 функции

public bool myMethod() {
    bool summaryCondition = myMethodWork();
    // do common code
    return summaryCondition;
}

private bool myMethodWork() {
   /* ... code ... */

    if(thisCondition) {
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        return false;
    }

    /* ... even more code ... */

    return lastCondition;
}
15 голосов
/ 09 февраля 2010

Это идеальное место для GOTO

* утки *

3 голосов
/ 09 февраля 2010

Ваше решение option C не далеко от оптимального, поскольку оно адекватно кодирует правильную последовательность выполнения, которую вы пытаетесь выполнить.

Аналогично, использование вложенных операторов if делает то же самое. Это может быть визуально менее привлекательным, но это проще для понимания и делает процесс выполнения довольно очевидным:

public bool myMethod() { 
    boolean  rc = lastCondition; 

    /* ... code-1 ... */ 

    if (thisCondition) { 
        rc = false;
    } 
    else {  
        /* ... code-2 ... */ 

        if (thatCondition) { 
            rc = false;
        } 
        else {  
            /* ... code-3 ... */ 
            rc = ???;
        }  
    }

    /* ... the code that must run at end of method ... */ 
    return rc;  
}

Упрощение Код дает:

public bool myMethod() { 
    boolean  rc = false; 

    /* ... code-1 ... */ 

    if (!thisCondition) { 
        /* ... code-2 ... */ 

        if (!thatCondition) { 
            /* ... code-3 ... */ 
            rc = lastCondition;
        }  
    }

    /* ... the code that must run at end of method ... */ 
    return rc;  
}

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

Чтобы ответить на ваш вопрос о try-finally блоках: Да, вы можете злоупотреблять ими. Ваш пример недостаточно сложен, чтобы оправдать использование try-finally. Если бы оно было более сложным, оно могло бы.

См. Мое мнение по этому вопросу: Перейти к утверждению, которое считается вредным: ретроспектива , "Обработка исключений" .

3 голосов
/ 09 февраля 2010

Если код должен выполняться даже при наличии Exception, то finally - это не просто хороший выбор, это необходимость. Если это не так, finally не требуется. Похоже, вы хотите найти формат, который «выглядит» лучше всего. Но здесь на карту поставлено немного больше.

2 голосов
/ 09 февраля 2010

Не злоупотребляйте try / finally, если вам не нужно вырваться из внутренних цикловЗлоупотребление делать / пока.

bool result = false;
do {
  // Code
  if (condition1) break;
  // Code
  if (condition2) break;
  // . . .
  result = lastCondition
} while (false);
2 голосов
/ 09 февраля 2010

Как насчет того, чтобы разбить его еще немного, чтобы дать что-то большее (исключая то, что я не использовал логические операторы Java в довольно долго ), как это:

public bool findFirstCondition()
{
   // do some stuff giving the return value of the original "thisCondition".
}

public bool findSecondCondition()
{
   // do some stuff giving the return value of the original "thatCondition".
}

public bool findLastCondition()
{
   // do some stuff giving the return value of the original "lastCondition".
}

private void cleanUp() 
{
   // perform common cleanup tasks.
}


public bool myMethod() 
{ 


   bool returnval = true;
   returnval = returnval && findFirstCondition();
   returnval = returnval && findSecondCondition();

   returnval = returnval && findLastCondition();
   cleanUp();
   return returnval; 
}
0 голосов
/ 09 февраля 2010

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

public boolean myMethod() {
    /* ... code ... */

    if(thisCondition) {
        return myMethodCleanup(false);
    }

    /* ... more code ... */

    if(thatCondition) {
        return myMethodCleanup(false);
    }

    /* ... even more code ... */

    return myMethodCleanup(lastCondition);
}

private boolean myMethodCleanup(boolean result) {

    /* ... the CODE that must run at end of method ... */
    return result;
}

это выглядит не очень хорошо, но это лучше, чем использование goto-подобных конструкций. Чтобы убедить своих товарищей по команде, что решение с 1 возвратом может быть не таким уж и плохим, вы также можете представить версию, используя 2 do { ... } while (false); и break (* evil smile *.)

0 голосов
/ 09 февраля 2010

IMO идея состоит в том, чтобы поместить блок try в небольшую часть кода (например, вызов метода), который может вызвать известное исключение (например, чтение из файла, чтение int в String) , Поэтому размещение блока try над всем кодом метода на самом деле не лучший способ, если только вы не ожидаете, что каждый код условия if потенциально вызовет один и тот же набор исключений. Я не вижу смысла в использовании блока try только ради использования finally.

Если я не ошибаюсь, размещение больших кусков кода внутри try также делает его намного медленнее, но не уверен, верно ли это в последних версиях Java.

Лично я бы выбрал вариант С. Но я ничего не имею против варианта А.

0 голосов
/ 09 февраля 2010

Есть ли причина, по которой вы не можете просто сохранить возвращаемое значение и выпасть из if?

   bool retVal = true;
   if (retVal && thisCondition) {
   }

   /* more code */

   if ( retVal ) {
     /* ... code that must run at end of method, maybe inside an if or maybe not... */

   }
   return retVal;
...