Это считают невоспитанностью для выполнения функции в условном операторе?

В строго типизированном языке, который не приравнивает 0 / null к false и отличное от 0 / non-null к true, я бы сказал, что это, вероятно, безопасно, но немного меньше читается в общем случае, когда имена ваших методов и количество параметров могут быть больше. Я бы лично избегал этого, за исключением некоторых стандартных идиом, в случаях, когда 0 / null приравнивается к false, а отличное от 0 / non-null - к true просто из-за потенциальной опасности запутать присваивание с проверкой равенства при чтении кода. Некоторые идиомы в языках со слабой типизацией, например C, настолько распространены, что избегать их не имеет смысла, например,

 while ((line = getline()) != null) {
   ...
 }

Проблема, как мне кажется, не в структуре, а в правилах вождения. Почему getOrange предшествует getApple и т. Д.?

Вероятно, вы увидите что-то более управляемое данными:

enum FruitEnum
{
  Orange, Apple, Mango, Banana
}

и отдельно,

List<FruitEnum> orderedFruit = getOrderedFruit();
int i = 0;
FruitObj selectedFruit;
while(selectedFruit == null && i <= orderedFruit.Count)
{
    fruit = FruitFactory.Get(orderedFruit[i++]);
}
if(fruit == null)
{
    throw new FruitNotFoundException();
}

Тем не менее, для упрощения кода вы можете использовать оператор объединения:

fruit = getOrange() ?? getApple() ?? getMango() ?? new Banana();

Прямо отвечу на ваш вопрос: обычно плохой тоном иметь побочные эффекты в условном выражении.

В качестве обходного пути вы можете сохранить конструкторы фруктов в массиве и найти первый конструктор, который возвращает ненулевое значение (псевдокод):

let constructors = [getOrange; getApple; getMango; fun () -> new Banana()]
foreach constructor in constructors
    let fruit = constructor()
    if fruit != null
        return fruit

Это похоже на оператор объединения с нулевым значением, но в более общем виде. В C # вы, вероятно, использовали бы linq следующим образом:

 var fruit = constructors
     .Select(constructor => constructor())
     .Filter(x => x != null)
     .First();

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

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

Я могу придумать две альтернативы.

Первый допустим только в таких языках, как ваш (PHP?), где single = в условном — это нормально.

    if ( (fruit = getOrange()) != null)
    elseif ( (fruit = getApple()) != null)
    elseif ( (fruit = getMango()) != null)
    else fruit = new Banana();

Дает понять, что вы проводите сравнение и что одинарные = не являются ошибкой.

    fruit = getOrange();
    if(!fruit) fruit = getApple();
    if(!fruit) fruit = getMango();
    if(!fruit) fruit = new Banana();

Так же, как и ваш второй пример, но избавляет от уродливой лишней вложенности.

В C или C++ вы могли бы написать:

return (fruit = getOrange()) ? fruit :
       (fruit = getApple())  ? fruit :
       (fruit = getMango())  ? fruit :
        new Banana();

Причина, по которой следует избегать и этого, и вашего первого варианта, не в "догме о базовых структурах", а в том, что присваивание само по себе в условии сбивает с толку. Во-первых, не все языки его поддерживают. С другой стороны, это легко понять как ==, или читатель может быть не уверен, действительно ли вы имели в виду это, или, возможно, вы имели в виду ==. Добавление != 0 к каждому условию становится довольно плотным и многословным.

GCC имеет расширение, позволяющее:

return getOrange() ? : getApple() ? : getMango() ? : new Banana();

То же самое часто можно сделать с помощью || или или (но не в C или C++).

Другая возможность:

do {
    fruit = getOrange();
    if (fruit) break;
    fruit = getApple();
    if (fruit) break;
    fruit = getMango();
    if (fruit) break;
    fruit = new Banana();
} while (false);

Это еще лучше работает в языке, где можно вырваться из основного блока, что можно сделать с помощью last в Perl, поскольку можно обойтись без do / while(false). Но, вероятно, это действительно понравится только программистам на ассемблере.