Однострочники и читабельность: когда прекратить сокращать код? [закрыто]

14

контекст

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

В настоящее время я в основном пишу на Ruby, поэтому я начал использовать линтер (Rubocop), чтобы предоставить мне некоторую информацию о «качестве» моего кода (это «качество» определяется проектом сообщества ruby-style-guide ).

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

Во всяком случае, делая все это, я понял (или, по крайней мере, вспомнил) несколько вещей:

  • Некоторые языки (особенно Python, Ruby и т. Д.) Позволяют создавать отличные однострочные коды
  • Следование некоторым рекомендациям для вашего кода может сделать его значительно короче и, тем не менее, очень четким
  • Тем не менее, строгое соблюдение этих рекомендаций может сделать код менее понятным / легким для чтения.
  • Кодекс может почти полностью соответствовать некоторым рекомендациям и при этом быть низкого качества.
  • Удобочитаемость кода в основном субъективна (например, «то, что я нахожу понятным, может быть совершенно неясным для коллег-разработчика»)

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

Теперь несколько примеров, чтобы прояснить все это.

Примеры

Давайте рассмотрим простой пример использования: у нас есть приложение с Userмоделью " ". У пользователя есть необязательный firstnameи surnameи обязательный emailадрес.

Я хочу написать метод " name", который потом вернет name ( firstname + surname) пользователя, если хотя бы его firstnameили surnameприсутствует, или егоemail в качестве запасного значения, если нет.

Я также хочу, чтобы этот метод принял " use_email" в качестве параметра (логическое значение), позволяя использовать электронную почту пользователя в качестве запасного значения. Этот use_emailпараметр " " должен по умолчанию (если не передан) как "true ".

Самый простой способ написать это в Ruby:

def name(use_email = true)
 # If firstname and surname are both blank (empty string or undefined)
 # and we can use the email...
 if (firstname.blank? && surname.blank?) && use_email
  # ... then, return the email
  return email
 else
  # ... else, concatenate the firstname and surname...
  name = "#{firstname} #{surname}"
  # ... and return the result striped from leading and trailing spaces
  return name.strip
 end
end

Этот код является наиболее простым и понятным способом сделать это. Даже для того, кто не «говорит» на Ruby.

Теперь давайте попробуем сделать это короче:

def name(use_email = true)
 # 'if' condition is used as a guard clause instead of a conditional block
 return email if (firstname.blank? && surname.blank?) && use_email
 # Use of 'return' makes 'else' useless anyway
 name = "#{firstname} #{surname}"
 return name.strip
end

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

Теперь давайте используем магию Ruby, чтобы сделать ее еще короче:

def name(use_email = true)
 return email if (firstname.blank? && surname.blank?) && use_email
 # Ruby can return the last called value, making 'return' useless
 # and we can apply strip directly to our string, no need to store it
 "#{firstname} #{surname}".strip
end

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

Именно здесь мы можем начать задавать вопрос: стоит ли оно того? Должны ли мы сказать «нет, сделать его читабельным и добавить»return « »(зная, что это не будет соответствовать рекомендациям). Или мы должны сказать: «Все в порядке, это путь Руби, учите этот чертов язык!»?

Если мы выберем вариант B, то почему бы не сделать его еще короче:

def name(use_email = true)
 (email if (firstname.blank? && surname.blank?) && use_email) || "#{firstname} #{surname}".strip
end

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

Мы также можем написать это:

def name(use_email = true)
 (email if [firstname, surname].all?(&:blank?) && use_email) || "#{firstname} #{surname}".strip
end

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

Вопрос

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

Так вот реальный вопрос: где остановиться? Когда коротко, достаточно коротко? Как узнать, когда код становится «слишком коротким» и менее читаемым (имея в виду, что он довольно субъективен)? И даже больше: как всегда соответствующим образом кодировать и избегать смешивания однострочников с «полноразмерными» кусками кода, когда мне просто хочется?

TL; DR

Главный вопрос здесь: когда дело доходит до выбора между «длинным, но ясным, читаемым и понятным фрагментом кода» и «мощным, более коротким, но трудным для чтения / понимания однострочным», зная, что эти два - верх и дно шкалы, а не два единственных варианта: как определить, где находится граница между «достаточно ясно» и «не так ясно, как должно быть»?

Главный вопрос не в классической «Однострочник против читабельности: какой из них лучше?» но "Как найти баланс между этими двумя?"

Редактировать 1

Комментарии в примерах кода предназначены для «игнорирования», они здесь для того, чтобы прояснить, что происходит, но не должны приниматься во внимание при оценке читабельности кода.

Sudiukil
источник
7
Слишком коротко для ответа: продолжайте рефакторинг итеративно, пока вы не будете уверены, что он лучше предыдущей итерации, затем остановите и отмените последнюю рефакторизацию.
Дом
8
Я бы предпочел вариант 3 с returnдобавленным ключевым словом . Эти семь символов добавляют немного ясности в мои глаза.
cmaster - восстановить монику
2
Если вы чувствуете себя действительно ужасно, вы можете написать все это как [firstname,surname,!use_email].all?(&:blank?) ? email : "#{firstname} #{surname}".strip... потому что false.blank?возвращает true и троичный оператор сохраняет вам несколько символов ... ¯ \ _ (ツ) _ / ¯
DaveMongoose
1
Хорошо, я должен спросить: какую ясность returnдолжно добавить ключевое слово ?! Он не предоставляет никакой информации вообще . Это чистый беспорядок.
Конрад Рудольф
2
Представление о том, что краткость порождает ясность, не только страдает от закона убывающей отдачи, но и переворачивается, когда доводится до крайности. Если вы переписываете, чтобы сделать короткую функцию короче, вы теряете время, и то же самое касается попыток оправдать практику.
sdenham

Ответы:

26

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

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

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

Другая объединенная особенность и идиома - использование конечных ifоператоров для повышения читабельности кода. Одна из движущих идей в Ruby - написать код, который читается на естественном языке. Для этого мы пойдем в «Заостренное руководство по Ruby», глава 3 .

Прочитайте вслух следующее для себя.

5.times { print "Odelay!" }

В английских предложениях знаки препинания (такие как точки, восклицания, круглые скобки) молчат. Пунктуация добавляет смысл словам, помогает подсказывать, что автор подразумевает под предложением. Итак, давайте прочитаем вышеизложенное как: пять раз напечатайте «Odelay!».

Учитывая это, пример кода № 3 наиболее идиоматичен для Ruby:

def name(use_email = true)
  return email if firstname.blank? && surname.blank? && use_email

  "#{firstname} #{surname}".strip
end

Теперь, когда мы читаем код, он говорит:

Возврат электронной почты, если имя не указано, а фамилия не указана и используйте адрес электронной почты

(возвращение) имя и фамилия

Что чертовски близко к реальному коду Ruby.

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

Грег Бургардт
источник
Хороший вопрос. Это правда, что этот вопрос не должен был быть ориентирован на Ruby, но я согласен, что здесь невозможно получить ответ, независимый от языка.
Судюкиль
8
Я нахожу идею сделать код звучащим, как естественный язык, сильно переоцененным (а иногда даже проблематичным). Но даже без этой мотивации я прихожу к тому же выводу, что и этот ответ.
Конрад Рудольф
1
Есть еще один твик, который я хотел бы рассмотреть для кода. То есть ставить use_emailперед другими условиями, так как это переменная, а не вызов функции. Но с другой стороны, интерполяция строк в любом случае перекрывает разницу.
Джон Дворак
Структурирование кода в соответствии со структурами естественного языка может привести к попаданию в языковые ловушки. Например, когда вы читаете следующие требования do send an email if A, B, C but no D, следуя вашему предположению, было бы естественно напечатать 2 блока if / else , когда, вероятно, было бы легче кодировать if not D, send an email. Будьте осторожны в момент чтения естественного языка и преобразуйте его в код, потому что он может заставить вас написать новую версию «Бесконечной истории» . С классами, методами и переменными. Ничего страшного в конце концов.
Laiv
@Laiv: Создание кода, читаемого как естественный язык, не означает буквального перевода требований. Это означает написание кода таким образом, чтобы при чтении вслух он позволял читателю понять логику, не читая каждый бит кода, символа за символом, языковой конструкции для языковой конструкции. Если кодирование это if !Dлучше, это прекрасно, так как Dимеет многозначительное имя. И если !оператор теряется среди другого кода, то иметь вызываемый идентификатор NotDбыло бы целесообразно.
Грег Бургардт
15

Я не думаю, что вы получите лучший ответ, чем "используйте свое лучшее суждение". Короче говоря, вы должны стремиться к ясности, а не краткости . Часто самый короткий код также является самым ясным, но если вы сосредоточены только на достижении краткости, может пострадать ясность. Это явно так в последних двух примерах, что требует больше усилий для понимания, чем в предыдущих трех примерах.

Важным фактором является аудитория кода. Конечно, читаемость полностью зависит от человека, который читает. Знают ли люди, которых вы ожидаете прочитать код (помимо себя), идиомы языка Ruby? Ну, этот вопрос не то, на что случайные люди в Интернете могут ответить, это только ваше собственное решение.

JacquesB
источник
Я согласен с мнением аудитории, но это часть моей борьбы: поскольку мое программное обеспечение часто с открытым исходным кодом, аудитория может состоять как из новичков, так и из "Ruby Gods". Я мог бы сделать это простым, чтобы сделать его доступным для большинства людей, но это своего рода пустая трата преимуществ, предлагаемых языком.
Судюкил
1
Как кто-то, кто должен был взять на себя, расширять и поддерживать действительно ужасный код, ясность должна победить. Помните старую поговорку: напишите свой код, как если бы сопровождающий был мстительным Ангелом Ада, который знает, где вы живете и куда ходят ваши дети в школу.
августа
2
@ Судюкил: Это важный момент. Я предлагаю вам стремиться к идиоматическому коду в этом случае (т.е. предполагать хорошее знание языка), так как вряд ли новички будут в любом случае способствовать открытому исходному коду. (Или, если они это сделают, они будут готовы приложить усилия для изучения языка.)
JacquesB
7

Часть проблемы здесь заключается в том, «что такое читабельность». Для меня я смотрю на ваш первый пример кода:

def name(use_email = true)
 # If firstname and surname are both blank (empty string or undefined)
 # and we can use the email...
 if (firstname.blank? && surname.blank?) && use_email
  # ... then, return the email
  return email
 else
  # ... else, concatenate the firstname and surname...
  name = "#{firstname} #{surname}"
  # ... and return the result striped from leading and trailing spaces
  return name.strip
 end
end

И мне трудно читать, так как он полон «шумных» комментариев, которые просто повторяют код. Раздень их:

def name(use_email = true)
 if (firstname.blank? && surname.blank?) && use_email
  return email
 else
  name = "#{firstname} #{surname}"
  return name.strip
 end
end

и теперь это намного более читабельно. Затем, читая его, я думаю: «Хм, мне интересно, поддерживает ли Ruby троичный оператор? В C # я могу написать его так:

string Name(bool useEmail = true) => 
    firstName.Blank() && surname.Blank() && useEmail 
    ? email 
    : $"{firstname} {surname}".Strip();

Возможно ли что-то подобное в рубине? Прорабатывая ваш пост, я вижу:

def name(use_email = true)
 (email if (firstname.blank? && surname.blank?) && use_email) || "#{firstname} #{surname}".strip
end

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

def name(use_email = true)
 (email if (firstname.blank? && surname.blank?) && use_email) 
 || "#{firstname} #{surname}".strip
end

Теперь я счастлив. Я не совсем уверен в том, как работает синтаксис, но я могу понять, что делает код.

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

Хотя я бы сказал одно: остерегайтесь «умного кода», как в вашем последнем примере. Спросите себя, [firstname, surname].all?(&:blank?)добавляет ли что-нибудь что-то кроме того, чтобы заставить вас чувствовать себя умным, потому что это демонстрирует ваши навыки, даже если теперь это немного труднее читать? Я бы сказал, что этот пример вполне подходит для этой категории. Если бы вы сравнивали пять значений, я бы посчитал это хорошим кодом. Итак, опять же, здесь нет абсолютной линии, просто помните о том, чтобы быть слишком умным.

Итак, подведем итоги: удобочитаемость требует, чтобы вы знали свою аудиторию и нацеливались на ваш код соответствующим образом и писали сжатый, но понятный код; никогда не пишите "умный" код. Держите это коротким, но не слишком коротким.

Дэвид Арно
источник
2
Ну, я забыл упомянуть об этом, но комментарии должны были быть «проигнорированы», они здесь только для того, чтобы помочь тем, кто плохо знает Ruby. Действительный момент, хотя об аудитории я не думал об этом. Что касается версии, которая вас радует: если важна длина строки, то третья версия моего кода (та, которая содержит только одно выражение return) делает это и даже немного более понятна, нет?
Судюкиль,
1
@Sudiukil, не будучи разработчиком ruby, я обнаружил, что его труднее всего читать, и он не соответствовал тому, что я искал (с точки зрения другого языка), как «лучшее» решение. Однако для тех, кто знаком с тем фактом, что ruby ​​является одним из тех языков, который возвращает значение последнего выражения, он, вероятно, представляет собой самую простую и легкую для чтения версию. Опять же, это все о вашей аудитории.
Дэвид Арно,
Не Ruby-разработчик, но это имеет для меня гораздо больше смысла, чем ответ с наибольшим количеством голосов, который гласит: «Вот что я собираюсь вернуть [сноска: при определенном длинном условии]. Кроме того, вот строка, которая пришла поздно» на вечеринку." Логика, которая по сути является просто оператором case, должна быть написана как единое унифицированное выражение case, а не распределяться по нескольким, казалось бы, не связанным операторам.
Пол
Лично я пошел бы с вашим вторым блоком кода, за исключением того, что я бы объединил два оператора в вашей ветке else в один:return "#{firstname} #{surname}".strip
Paul
2

Вероятно, это вопрос, где трудно не дать основанный на мнении ответ, но вот мои два цента.

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

Так что было бы хорошей причиной? На самом деле, это суждение, но примером может быть что-то вроде оптимизации производительности (после тестирования производительности, конечно, не заранее). Что-то, что приносит вам некоторую пользу, за которую вы готовы платить с пониженной читабельностью. В этом случае вы можете уменьшить этот недостаток, предоставив полезный комментарий (который объясняет, что делает код, и почему его нужно было сделать немного загадочным). Более того, вы можете извлечь этот код в отдельную функцию со значимым именем, так что это всего лишь одна строка на сайте вызова, которая объясняет, что происходит (через имя функции), не вдаваясь в подробности (однако люди по-разному мнения об этом, так что это еще один суждение, которое вы должны сделать).

Филипп Милованович
источник
1

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

Каждое изменение должно улучшить способность среднего человека понимать код. Чтобы сделать код понятным, полезно использовать следующие рекомендации:

  • Уважайте идиомы языка . C #, Java, Ruby, Python имеют свои предпочтительные способы сделать то же самое. Идиоматические конструкции помогают понять код, с которым вы не знакомы.
  • Остановитесь, когда ваш код станет менее читабельным . В приведенном вами примере это произошло, когда вы нажали последние пары сокращающего кода. Вы потеряли идиоматическое преимущество предыдущего примера и ввели много символов, которые требуют много размышлений, чтобы по-настоящему понять, что происходит.
  • Используйте комментарии только тогда, когда вам нужно оправдать что-то неожиданное . Я знаю, что ваши примеры были там, чтобы объяснить конструкции людям, менее знакомым с Ruby, и это нормально для вопроса. Я предпочитаю использовать комментарии для объяснения неожиданных бизнес-правил и избегать их, если код может говорить сам за себя.

Тем не менее, бывают случаи, когда расширенный код помогает лучше понять, что происходит. Один пример с этим прибывает из C # и LINQ. LINQ является отличным инструментом и может улучшить читаемость в некоторых ситуациях, но я также сталкивался с рядом ситуаций, когда это было намного более запутанным. У меня была некоторая обратная связь в рецензировании, в которой предлагалось превратить выражение в цикл с соответствующими операторами if, чтобы другие могли лучше его поддерживать. Когда я согласился, они были правы. Технически, LINQ более идиоматичен для C #, но есть случаи, когда он ухудшает понятность, а более подробное решение улучшает его.

Я говорю все это, чтобы сказать это:

Улучшение, когда вы можете сделать свой код лучше (более понятным)

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

Берин Лорич
источник
0

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

Когда однострочники повышают читабельность, а когда вредят?

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

  1. Они слишком специфичны для извлечения из функции.
  2. Вы не хотите прерывать «поток» чтения окружающего кода.

Например, скажем, nameне было хорошего ... названия для вашего метода. То, что объединение имени и фамилии или использование электронной почты вместо имени, не было естественным делом. Так что вместо nameлучшего, что вы могли придумать, получилось долго и громоздко:

puts "Name: #{user.email_if_there_is_no_name_otherwise_use_firstname_and_surname(use_email)}"

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

Тем не менее - зачем делать это однострочным? Они обычно менее читаемы, чем многострочный код. Здесь мы должны проверить мое второе условие - поток окружающего кода. Что делать, если у вас есть что-то вроде этого:

puts "Group: #{user.group}"
puts "Title: #{user.title}"
if user.firstname.blank? && user.surname.blank?) && use_email
  name = email
else
  name = "#{firstname} #{surname}"
  name.strip
end
puts "Name: #{name}"
puts "Age: #{user.age}"
puts "Address: #{user.address}"

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

puts "Group: #{user.group}"
puts "Title: #{user.title}"
puts "Name: #{(email if (user.firstname.blank? && user.surname.blank?) && use_email) || "#{user.firstname} #{user.surname}".strip}"
puts "Age: #{user.age}"
puts "Address: #{user.address}"

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

Это твой случай? Точно нет!

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

Что касается второго условия - прерывает ли он поток окружающего кода? Нет! Окружающий код - это объявление метода, выбор которого nameявляется его единственной целью. Логика выбора имени не прерывает поток окружающего кода - это сама цель окружающего кода!

Вывод - не делайте все тело функции однострочным

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

Заметка

Я имею в виду «полноценные» функции и методы, а не встроенные функции или лямбда-выражения, которые обычно являются частью окружающего кода и должны вписываться в его поток.

Идан Арье
источник