стиль кода: множественные возвраты

Question

Я пишу метод по следующим направлениям:

if (hasFoo()) {
   return calculateFoo();
} else if (hasBar()) {
   return calculateBar();
} else {
   return calculateBaz();
}

Получатели довольно дороги, и проверки has...() либо дублируют большую часть логики, либо просто требуют повторного использования получателей.Я мог бы использовать методы has...(), чтобы сохранить результат get...() в поле и сделать получатель ленивым, но было бы неплохо, чтобы has...() не имел побочных эффектов.Я мог бы написать это с помощью вложенных блоков try {} catch {}, но это не выглядит элегантно.Похоже, что должно быть лучшее решение для этого ...

РЕДАКТИРОВАТЬ: изменил get...() на calculate...(), чтобы было ясно, что они дорогие.

Answer 1

int result = 0;

if (hasFoo()) {
   result = getFoo();
} else if (hasBar()) {
   result = getBar();
} else {
   result = getBaz();
}

return result;

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

Answer 2

Я не вижу в этом ничего плохого

Object fooBarBaz = null;

if (hasFoo()) {
   foo = getFoo();
} else if (hasBar()) {
   fooBarBaz = getBar();
} else {
   fooBarBaz = getBaz();
}

return fooBarBaz;

Answer 3

Вместо выполнения hasXXX() и calculateXXX() вы можете разделить эти вычисления на отдельные объекты, например

public interface CalculationModel {
    Object calculate();
}

public class FooCalculationModel implements CalculationModel {
    @Override
    public Object calculate() {
        // Perform Foo calculations
        return result;
    }
}

, и ваш оператор if может быть заменен на:

return getCalculationModel().calculate();

Конечно, вам понадобится какой-то способ определения CalculationModel, но тогда он заменит методы hasFoo (), hasBar () и т. Д.

Answer 4

Это не проблема "можно ли делать несколько возвратов" - ваши множественные возвраты в порядке.

Это проблема рефакторинга и / или состояния хранилища.

Если у вас есть:

bool hasXXX() {
    // do lots of stuff
    ...

    return has_xxx;
}

и

double calculateXXX() {
    // do the same lots of stuff
    ...

    // do some more stuff
    ...

    return xxx;
}

тогда сложность задачи зависит от того, дает ли вычисление hasXXX() множество промежуточных значений, необходимых для calculateXXX().

Вам, вероятно, нужно что-то вроде:

bool checked_xxx = false;
double xxx_state;

bool hasXXX() {
   // do expensive stuff
   ...

   // save temporary state variables
   xxx_state = ... 

   // remember that we've been here
   checked_xxx = true;

   // send back the required value
   return has_xxx;
}

double calculateXXX() {
    // make sure that hasXXX was called, and is valid
    if (!checked_xxx && !hasXXX()) {
        // should never happen - you called calculateXXX when hasXXX() == false
        throw new RuntimeException("precondition failed");
    }

    // use the previously calculated temporary state variables
    ...

    // send back the final result
    return xxx;
}

Answer 5

Я не уверен, что это ваш случай, но я постараюсь полностью изменить код. В настоящее время, насколько я понимаю, ваш код выглядит примерно так (пример):

boolean hasFoo() {
    DataObject do = getSomeDataSource().getSomeDataObject();
    if (do.getF() != null && do.getO() != null) {
        return true;
    } else {
        return false;
    }
}

Foo getFoo() {
    DataObject do = getSomeDataSource().getSomeDataObject();
    Foo result = new Foo(do.getF(), do.getO());
    return result;
}

В основном здесь происходит то, что один и тот же код используется для проверки возможности возврата Foo и для создания самого Foo. И я бы рефакторинг это к этому:

/**
* @returns instance of Foo or null if Foo is not found
*/
Foo getFoo() {
    DataObject do = getSomeDataSource().getSomeDataObject();
    F f = do.getF();
    if (f == null) {
        return null; //Foo can not be created
    }
    O o = do.getO();
    if (o == null) {
        return null; //Foo can not be created
    }

    return new Foo(f,o);
}

Теперь ваш исходный код станет похож на это:

Result r;

r = getFoo();
if (r == null) {
    r = getBoo();
}
if (r == null) {
    r = getDoo();
}
return r;

Answer 6

Я предпочитаю это так:

if (hasFoo()) {
    return calculateFoo();
}

if (hasBar()) {
    return calculateBar();
}

return calculateBaz();

Все дело вкуса и условностей.

Answer 7

РЕДАКТИРОВАТЬ: Если я правильно интерпретирую ваши комментарии, это звучит так, как будто вы на самом деле хотите что-то вроде:

Result result = calculateFoo();
if (result != null) {
    return result;
}
result = calculateBar();
if (result != null) {
    return result;
}
return calculateBaz();

... где каждый из методов calculate возвращаетnull, если соответствующий метод has возвращает false.Теперь, если null является действительным «реальным» возвращаемым значением, вы всегда можете обернуть результат так, чтобы calculateFoo возвращало значение, которое может в основном сказать: «Да, у меня есть действительное значение, и это X» или «нет»., У меня нет действительного значения "(тип" возможно ").

---

Оригинальный ответ

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

Вы ясно даете понять, что, как только вы дойдете до каждой из «листовых» частей,вы точно знаете, что является возвращаемым значением, и единственный другой код, который должен быть выполнен перед выходом из метода, - это любой код очистки в finally блоках.

Наличие единой точки выхода имеет смысл в языках, которыене имею try / finally или GC (где вы действительно хотите убедиться, что вы делаете всю очистку в одном месте), но в Java, я думаю, что возвращение, когда вы знаете результат, выражает ваше намерение более четко, чем использование отдельной локальной переменной.

Сказав это, еще одной альтернативой рассмотрим использование условного оператора, раскладывающего ваш код, так что он, очевидно, проходит серию тестов и возвращает, как только обнаружит первый "match ":

return hasFoo() ? getFoo()
     : hasBar() ? getBar()
     : getBaz();

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

Answer 8

вы могли бы сделать что-то вроде этого:

Object bar;
if ((bar = getFoo()) != null) {
  return bar;
} else if ((bar = getBoo()) != null) {
  return bar;
} else {
  return getBaz()
}

таким образом, вам нужно только вызвать методы get, но не методы has

EDIT

это то же самое в более читаемом формате, что также устраняет необходимость вызова методов has

Object bar = getFoo()

if (bar == null) {
  bar = getBoo()
}

if (bar == null) {
  bar = getBaz()
}

return bar;