Проще ли группировать тесты или держать их отдельно?

Question

Я только что поговорил с коллегой, где мы не согласились с тем, какой из следующих фрагментов проще:

public boolean foo(int x, int y) {
    if (x < 0)
        return false;
    if (y < 0)
        return false;
    // more stuff below
}

ИЛИ

public boolean foo(int x, int y) {
    if (x < 0 || y < 0)
        return false;
    // more stuff below
}

Очевидно, что короче;также очевидно, что их цикломатическая сложность идентична (поэтому для этого определения «простой», конечно, они одинаковы).

Каково ваше чувство и почему?Который является более читабельным;что легче отлаживать?

Answer 1

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

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

Например,

if (user.isDisabled() || user.isSuspended())
    return false;

оба теста касаются того, может ли пользователь что-то сделать.

, но

if (user.isDisabled())
    return false;
if (catalog.isClosedForOrders()) 
    return false;

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

Answer 2

Ну, во многом это будет зависеть от моего настроения в то время.

Я думаю, что наиболее объективным соображением будет то, насколько тесно связаны x & y.Проверка одной переменной для верхнего и нижнего диапазона - одна, если.Проверьте строку на null и int на диапазон - два ifs.

Answer 3

Лучше имена, тогда это действительно ясно ...

public boolean foo(int x, int y) {
    var hasInvalidArgument = (x < 0 || y < 0);

    if (hasInvalidArgument)
        return false;

    // more stuff below
}

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

Answer 4

Я думаю, что стили предлагают разные вещи:

if (x < 0)
    return false;
if (y < 0)
    return false;

говорит, что что-то сделано, если x < 0 и еще одна вещь сделана, если y < 0.

Тогда как

if (x < 0 || y < 0)
    return false;

подразумевает, что одна вещь выполнена, если выполняется одно из условий.

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

Answer 5

Я бы пошел на второй, с одним изменением:

public boolean foo(int x, int y) {
    if ((x < 0) || (y < 0))
        return false;
    // more stuff below
}

Причина в том, что оно следует тому, как человек произносит его на естественном языке (мы говорим «если какой-то из двух отрицателен, тогда ответ« нет »», а не «если первый отрицательный ответ - «нет»; если второй отрицательный, ответ также «нет»).