Много составляющих в if - - Признак плохого кода #6

10 Декабря 2020 Код
3 3

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

func foo (something) {
   if (something.isActive &&
      something.StartDate < DateTime.Now && 
     (something.EndDate == null ||  something.EndDate > DateTime.Now) &&
     User.CurrentUser == something.Creator && canEdit(something)) {
   }
}

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

func foo (something) {
   bool isActive = something.isActive && something.StartDate < DateTime.Now && 
     (something.EndDate == null ||  something.EndDate > DateTime.Now);

   bool hasAccess =   User.CurrentUser == something.Creator && canEdit(something);

   if (isActive && hasAccess) {
   }
}

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

А можно создать свойства или методы:

bool isActive(something) {
  return something.isActive && something.StartDate < DateTime.Now && 
      (something.EndDate == null ||  something.EndDate > DateTime.Now);
}

bool hasAccess(something) {
 return User.CurrentUser == something.Creator && canEdit(something);
}

func foo (something) {
   if (isActive(something) && hasAccess(something)) {
   }
}

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

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

// check if it is active and if we have access
func foo (something) {
   if (something.isActive &&
      something.StartDate < DateTime.Now && 
     (something.EndDate == null ||  something.EndDate > DateTime.Now) &&
     User.CurrentUser == something.Creator && canEdit(something)) {
   }
}

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

Понравилось? Кликни Лайк, чтобы я знал, какой контент более интересен читателям. Заметку уже лайкнули 3 человек