Как распознать, что короткие блоки кода могут быть преобразованы в нечто более чистое?

Question

У меня есть немного кода, который я написал несколько недель назад (цель кода не так важна, как его структура):

if (_image.Empty)
{
   //Use the true image size if they haven't specified a custom size
   if (_glyphSize.Width > 0)
      imageSize.Width = _glyphSize.Width //override
   else
      imageSize.Width = _image.GetWidth;

   if (_glyphSize.Height > 0) then
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height = _image.GetHeight
}
else
{
   //No image, but they can still override it with a custom size
   if (_glyphSize.Width > 0) then
      imageSize.Width = _glyphSize.Width
   else
      imageSize.Width = 0;

   if (_glyphSize.Height > 0)
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height := 0;
}

Сегодня вечером я перебирал это, и, когда я чистил его, я понял, что очищенная версия должна быть более лаконичной:

//Figure out the final image width
if (_glyphSize.Width > 0)
   imageSize.Width = _glyphSize.Width
else if (not _glyph.Empty)
   imageSize.Width = _glyph.GetWidth
else
   imageSize.Width = 0;

//Figure out the final image height
if (_glyphSize.Height > 0)
   imageSize.Height = _glyphSize.Height
else if (not _glyph.Empty)
   imageSize.Height = _glyph.GetHeight
else
   imageSize.Height = 0;

Примечание: Я урезал код до чистого логического потока и запутал исходный язык.

В конце концов я взял вложенные if и перевернул их. Это позволило сократить это. У меня вопрос: как я могу распознать это в будущем?

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

---

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

• если у них есть разрешение, они могут это сделать
• если у них нет разрешения, но действует переопределение

Который я изначально закодировал как:

if ((HasPermission || (!HasPermission and OverrideEnabled))
{
   ...do stuff
}

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

Permission  Override   Result
    0           0        0
    0           1        1
    1           0        1
    1           1        1

Что, когда я смотрю на это, является ИЛИ операцией. Итак, мое if утверждение стало:

if (HasPermission  or OverrideEnabled)
{
   ...
}

Что очевидно и просто. И вот теперь мне интересно, как я не мог увидеть это с самого начала.

---

Что возвращает меня к моему такому вопросу: какие контрольные знаки можно / нужно искать, чтобы распознать, что некоторый блок кода нуждается в некотором TLC?

Answer 1

Вот несколько советов из Code Complete, в верхней части моей головы. Это хорошая книга для такого рода вещей.

1. Вложенные if-else и повторяющиеся операторы в блоках
2. Длинные петли для
3. Повторяющиеся строки / операторы или часто используемые операции могут быть помещены в функцию
4. Если по каким-то причинам вы копируете и вставляете строку кода снова и снова

Я обнаружил, что дискретная математика влияет на то, как я сейчас пишу операторы if. Обычно я вижу, что пишу два одинаковых оператора IF в 2 блоках, а затем выполняю некоторый умственный «факторинг».

Answer 2

Молодец. Теперь, когда я вижу это:

//Figure out the final image width
if (_glyphSize.Width > 0)
...

//Figure out the final image height
if (_glyphSize.Height > 0)
...

Я думаю, что рефакторинг еще предстоит сделать. Извлечение кода в методы - это не просто отличный способ устранить избыточный код. Это также отличный способ сделать самодокументированный код:

Я бы хотел уменьшить код до:

set_final_image_size

С set_final_image_size и его миньонами, определенными так:

def set_final_image_size:
  imageSize.Width = final_image_width;
  imageSize.Height = final_image_height;

def final_image_width:
  if (_glyphSize.Width > 0)
     return _glyphSize.Width;
  else if (not _glyph.Empty)
     return _glyph.GetWidth;
  else
     return 0;

def final_image_height:
  if (_glyphSize.Height > 0)
     return _glyphSize.Height;
  else if (not _glyph.Empty)
     return _glyph.GetHeight;
  else
     return 0;

Answer 3

В частности, что касается булевой оценки, стоит отметить, что большинство (?) Современных языков реализуют ленивую оценку.

То есть, если «а» истинно, то if(a) и if(a or b) логически и функционально эквивалентны; интерпретатор прекращает оценку, когда видит or после истинной переменной. Это не очень важно, когда a и b являются переменными, но если они могут вызываться [например, if(a() or b())], b() не будет оцениваться, если a истинно.

Вы можете сэкономить много нажатий клавиш (и процессорного времени), изучив это хорошо:

if(!userExists()):
    if(!createUser()):
        errorHandling()
    else:
        doStuff()
else: doStuff()

становится

if(userExists() or createUser()): doStuff()
else: errorHandling()

Answer 4

Теперь, когда вы разделили логику ширины и высоты и заметили, что она идентична - что если вы добавите, скажем, getDimension(Direction direction) и setDimension(Direction direction, int length) к своим классам? Теперь у вас есть

if (_glyphSize.getDimension(direction) > 0)
   imageSize.setDimension(direction, _glyphSize.getDimension(direction))
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

Извлечение местного приносит нам:

length = _glyphSize.getDimension(direction);
if (length > 0)
   imageSize.setDimension(direction, length)
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

немного дальше:

length = _glyphSize.getDimension(direction);
if (length == 0 && !_glyph.Empty)
    length = _glyph.getDimension(direction);
imageSize.setDimension(direction, length);

Что, по крайней мере, на мой взгляд, начинает выглядеть довольно мило.