Если вы используете форматирование выписок, то каков ваш стиль? - PullRequest
2 голосов
/ 26 ноября 2008

Глядя на улучшение моего утверждения IF, и я хочу, чтобы мой код выглядел красиво

Это то, что я сейчас делаю, это читабельно , есть место для улучшения ?

SomeObject o = LoadSomeObject();


if( null == o
    ||
    null == o.ID || null == o.Title
    ||
    0 == o.ID.Length || 0 == o.Title.Length
 )

Мне не с кем поспрашивать. Вот почему я пришел сюда в первую очередь. Пожалуйста, не закрывайте мой вопрос :(

Ответы [ 12 ]

12 голосов
/ 26 ноября 2008

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

if (value1 == value2 ||
    value3 == value4 ||
    value5 == value6 ||
    value7 == value8) {

  executeMyCode();
}
10 голосов
/ 26 ноября 2008

Ваше многословие приводит к менее читаемому коду, я думаю, что следующий формат лучше всего:

if ( null == o || null == o.ID || null.Title || 0 == o.ID.Length || 0 == o.Title.Length )
{
  // do stuff
}

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

if ( IsIDEmpty(o) )
{
  // do stuff
}

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

6 голосов
/ 26 ноября 2008

Для простейшего форматирования того, что у вас есть, я бы выбрал по одному на строку.

if(null == o
  || null == o.ID
  || null == o.Title
  || 0 == o.ID.Length
  || 0 == o.Title.Length)

Еще лучше было бы, если бы вы могли рефакторизовать условие так, чтобы оно помещалось на одной строке. Я считаю, что большое количество || или && обычно трудно читать. Возможно, вы можете преобразовать его в функцию и оставить:

if(myFunction(...))
4 голосов
/ 27 ноября 2008

Мое эмпирическое правило: избегайте форматов, которые полумудрый автоформатер не может воспроизвести.

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

И если ваш код по-прежнему не читается после его автоматического форматирования, то есть вероятность, что вам все равно придется провести рефакторинг.

3 голосов
/ 26 ноября 2008

Я думаю, что это довольно нечитаемо.

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

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

if (o == null       ||
    o.ID == null    || o.ID.length == 0 ||
    o.Title == null || o.Title.Length == 0) 
3 голосов
/ 26 ноября 2008

Я бы поместил все это в одну строку, если она подходит (чего явно нет). С этим я бы поставил || последовательно в начале или конце строки:

if( null == o ||
    null == o.ID ||
    null == o.Title ||
    0 == o.ID.Length ||
    0 == o.Title.Length
)

или

if( null == o
    || null == o.ID
    || null == o.Title
    || 0 == o.ID.Length
    || 0 == o.Title.Length
)

Вы можете иметь> 1 условие в строке, расположение || Я думаю, это важнее.

Я игнорирую тот факт, что null. Название не имеет особого смысла

2 голосов
/ 27 ноября 2008

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

Например:

Стандарты кодирования GNU
http://www.gnu.org/prep/standards/

Соглашения о коде для языка программирования Java
http://java.sun.com/docs/codeconv/

Общие рекомендации по проектированию .NET Framework для разработчиков библиотек классов
http://msdn.microsoft.com/en-us/library/czefa0ke.aspx

2 голосов
/ 26 ноября 2008

Я нахожу это довольно отвлекающим, если честно. Главным образом потому, что '||' начать делать смешные скороговорки.

Я предпочитаю что-то вроде

if ( o == null || o.ID == null || null.Title ||
     o.ID.Length == 0 || o.Title.Length )

или даже лучше, держите его в одной строке, если это возможно.

1 голос
/ 27 ноября 2008

Обычно я работаю с TravisO, но если в вашем операторе if () так много условий, что он просто долго сходит с ума, подумайте над тем, чтобы вместо этого добавить собственную маленькую функцию:

bool wereTheConditionsMet()
{
  if( NULL == 0 )
    return true;
  if( NULL == o.ID )
    return true;
  :  :   // and so on until you exhaust all the affirmatives
  return false;
}

if ( wereTheConditionsMet() )
{
  // do stuff
}

Намного легче передать намерение хорошо названной функции предиката, чем бесконечную строку из || s и && s.

0 голосов
/ 27 ноября 2008
  1. Используйте автоматический форматировщик кода и настройте его соответствующим образом.
  2. Напишите метод наподобие isPresent (String), который проверяет аргумент String на ненулевое и не пустое (нулевая длина).
  3. Переписать исходное условное выражение для использования нового метода isPresent (String), возможно, все в одну строку.
...