С чего лучше всего начинать рефакторинг?

Question

[HarrySto]

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

function changePrice($this = null) {
  if ($this == null) {
      var rel_price =  $('.all_colors_preview.active#rel_colors').find('.item.active .price').attr('data-price'),
          option_price = $('.all_colors_preview.active#option_colors').find('.item.active .option-price').val(),
          prefix = $('.all_colors_preview.active#option_colors').find('.item.active .option-price-prefix').val(),
          price_current = $('.price_wrapper .price_current'),
          option_sizes = $('.option_size.active'),
          quantity = Number($('#inner .quantity_val').text());
  } else {
    var rel_price =  $this.find('#rel_colors.active .item.active .price').attr('data-price'),
        option_price = $this.find('#option_colors.active .item.active .option-price').val(),
        prefix = $this.find('#option_colors.active .item.active .option-price-prefix').val(),
        price_current = $('.price_wrapper .price_current'),
        option_sizes = $this.find('.option_size.active'),
        quantity = Number($this.find('.quantity_val').text());
  }

  if (!quantity) {
    quantity = Number($('#inner .quantity_val').text());
  }

  var current = Number(price_current.attr('data-original-price'));  

  if (rel_price != '' && rel_price != undefined) {
    price_current.attr('data-update-price', rel_price);
    updated = Number(price_current.attr('data-update-price')) * quantity;
    updated = String(updated).replace(/(\d)(?=(\d\d\d)+([^\d]|$))/g, '$1 ')
    price_current.text(updated + ' ₽');
  } else if (option_price != '' && option_price != undefined && prefix != '') {
    if (prefix == '+') {
      update_price = current + Number(option_price);
    } else {
      update_price = current - Number(option_price);
    }

    price_current.attr('data-update-price', update_price);
    update_price = quantity * price_current.attr('data-update-price');
    price_current.text(String(update_price).replace(/(\d)(?=(\d\d\d)+([^\d]|$))/g, '$1 ') + ' ₽');
  } else if (option_price == '') {
    price_current.attr('data-update-price', current);
    current *= quantity;
    price_current.text(String(current).replace(/(\d)(?=(\d\d\d)+([^\d]|$))/g, '$1 ') + ' ₽');
  } else {
    return false;
  }

  if (option_sizes && option_sizes.length) {
    if (price_current.attr('data-update-price')) {
      current = Number(price_current.attr('data-update-price')); 
    }
    var option_prefix = option_sizes.find('input[type="hidden"]').attr('data-prefix'),
        option_size_price = option_sizes.find('input[type="hidden"]').attr('data-price');
    if (option_prefix == '+') {
      update_price = current + Number(option_size_price);
    } else {
      update_price = current - Number(option_size_price);
    }

    price_current.attr('data-update-price', update_price);
    update_price = quantity * price_current.attr('data-update-price');
    price_current.text(String(update_price).replace(/(\d)(?=(\d\d\d)+([^\d]|$))/g, '$1 ') + ' ₽');
  }

Пытался найти сервис для автоматического рефакторинга или анализа кода, ничего не нашел.

Comments

[HarrySto]

Скажите а если нужно седлать код уникальным? Есть какие-то сервисы, которые помогают сделать это автоматически? Мне советовали как сервисы AppRefactoring и CodeSnece

Answer 1

[mayton2019]

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

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

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

.replace(/(\d)(?=(\d\d\d)+([^\d]|$))/g, '$1 ')

встречается 4 раза. Ее можно обработать через extract function. После этого кода станет визуально меньше.

Далее - повторяющийся code-block с обновлением update_price. Его брат-близнец отличается только
аргументом option_price. На этом можно сыграть и тоже сделать некоторое упрощение.

if (prefix == '+') {
      update_price = current + Number(option_price);
    } else {
      update_price = current - Number(option_price);
    }

Некоторые фрагменты кода напрашиваются на introduce temporary variable.
Например следующий сниппет
option_sizes.find('input[type="hidden"]')
был использован дважды. И первый раз он нашел какое-то значение и был использован.
И в следующей строке этот поиск был вызван снова. Хотя операция find как-бы подсказывает
нам что она имеет complexity близкое к линейному. Тоесть нужно ценить процессорное
время и не грузить его повторными поисками. Тут - как-бы смысл двойной. И перформанс
и нормализация кода. Тоесть явное указание что этот результат нам еще понадобиться.

Очень сильно в рефакторинге помогает типизация. Тоесть если переписать код с JavaScript на TypeScript
то будут более очевидны некоторые инварианты.

Вот на этом пожалуй хватит. Надо сделать эти рефакторинги и возможно дальше откроется видение других.

Comments

[szQocks]

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

[mayton2019]

szQocks, про очко - это было грубо.

Я на самом деле не программирую ни на JS ни на TS. Но фронтендеров мне реально жалко.

[Igor Kalashnikov]

szQocks, и какие-же минусы в TS? учитывая, что на нем можно писать обычный JS код