Логическое задание для лучшей практики [закрыто]

10

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

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Я считаю, что этот код является избыточным и безобразным, поэтому я изменил его на то, что мне показалось простым логическим назначением, основанным на сравнении:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

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

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE) ? true : false;

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

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

Зак Постен
источник
25
Третья форма смешна; это просто говорит о том, что уже должно быть очевидно во второй форме.
Роберт Харви
6
Это полностью зависит от личных предпочтений. Мы можем притворяться иначе, но поскольку все они функционально эквивалентны, это сводится к стилю . Конечно, есть разница в удобочитаемости, но мой «читаемый и прозрачный» может быть вашим «тупым и непрозрачным»
MetaFight
3
@scriptin 5-8 строк v 1 строка - это больше, чем предпочтение, 5-8 строк обычно понятнее и лучше для этого. В этом простом примере я предпочитаю 1 линию, но в целом я видел слишком много 10 вкладышей, которые были скрыты в 1 вкладыш для удобства. Учитывая это, я бы никогда не пожаловался на вариант 1, он может быть не очень красивым, но он делает свою работу так же ясно и очевидно.
gbjbaanb
4
Варианты 1 и 3 говорят мне: «Автор не совсем понимает логическую логику».
17 из 26
2
Вариант 1 может быть полезен, если вам часто приходится устанавливать точку останова, которая зависит от значения.
Ян

Ответы:

39

Я предпочитаю 2, но я мог бы пойти на небольшую корректировку:

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

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

Джейкоб Райл
источник
4
это правильный ответ - хотя код в вопросе правильный, добавление скобок говорит читателю, что это не как задание. Если вы быстро просматривали код, скобки дают вам мгновенную дополнительную информацию, которая останавливает вас, чтобы посмотреть, является ли код таким, и не был ли это случайной ошибкой. Например, представьте, что строка была a = b == c, вы имели в виду назначить bool или вы имели в виду назначить c для b и a.
gbjbaanb
Скобки предотвратят двойное назначение в Python. Даже в языках, где они не предотвращают двойное назначение, круглые скобки определенно помогают указать, что вы имеете дело с двумя видами операций.
user2357112 поддерживает Monica
23

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

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

Вариант 3 сочетает в себе недостатки как 1, так и 2.

Килиан Фот
источник
Ну, вариант 1 разделяет свое преимущество с вариантом 2 ...
Дедупликатор
1
+1 за инфантильный код. Я искал такой код годами, мне просто не хватало правильного слова для его идентификации.
Лилиенталь
1
Мое первое предположение с кодом, подобным Варианту 1, состоит в том, что две ветви в одной точке в прошлом были более сложными, и кто-то не обращал внимания при рефакторинге. Если, однако, именно так было при первом написании, я согласен с «непониманием логических
выражений
13

В любое время код является более сложным, чем он должен вызывать "что это должно делать?" запах в читателе.

Например, первый пример заставляет меня задуматься: «Были ли другие функции в операторе if / else в какой-то момент, который был удален?»

Пример (2) прост, понятен и делает именно то, что нужно. Я прочитал это и сразу понял, что делает код.

Дополнительный пух в (3) заставил бы меня задуматься, почему автор написал это так (2). Там должна быть причина, но в данном случае не кажется , нет, так что это не полезно вообще и труднее читать , потому что синтаксис предполагает , что - то настоящее , которое не существует. Попытка узнать, что присутствует (когда ничего нет) затрудняет чтение кода.

enderland
источник
2

Легко видеть, что Вариант 2 и Вариант 1 связаны посредством ряда очевидных и простых рефакторингов:

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Здесь у нас есть ненужное дублирование кода, мы можем выделить назначение:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE)
{
    true
}
else
{
    false
}

или написано более кратко:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE) true else false

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

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE

Варианты 1 и 3 - это типичный код новичка, написанный кем-то, кто не понимает, что такое возвращаемое значение сравнения.

Йорг Миттаг
источник
Я бы добавил вашу if (...) ... ложную часть в качестве комментария непосредственно перед хорошим, тогда вы получите простое сканирование через ясность кода и лучший код.
DaveM
2

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

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

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

Это очень понятно для любого, кто знает синтаксис C / C ++ / C # / Java / Javascript.

Это также намного более читабельно, чем 8 строк.

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

и менее подвержен ошибкам

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.Needschange = false;
}

И это лучше, чем добавлять ненужные символы, как будто вы наполовину забыли синтаксис языка:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE ? true : false;

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE);

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

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


Как правило, первое, что делает код Real World TM нечитаемым, это его количество.

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

Пол Дрэйпер
источник
1

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

Вы можете улучшить читабельность, добавив пробелы и фигурные скобки, такие как:

obj.NeedsChange =    obj.Performance <= LOW_PERFORMANCE;

или же

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

как упомянул Джейкоб Рэйл.

Удай Шанкар
источник