Иногда человеческое тело занимается, казалось бы, бессмысленной деятельностью, называемой «бесполезной ездой на велосипеде», когда два метаболических пути протекают одновременно в противоположных направлениях и не имеют общего эффекта, кроме как рассеивать энергию в форме тепла ( википедия ).
Рефакторинг, выполненный в предыдущих двух статьях, может показаться бесполезным. У нас есть около 100 строк запутанного кода, но это было много работы без видимого общего эффекта.
Текущее состояние можно наблюдать на уровне модели, более или менее изолированном там, где он находится на уровне модели:
module Stats class RunningTimeData include Rails.application.routes.url_helpers SECONDS_PER_DAY = 86400; attr_reader :title, :actions_running_time, :key, :today, :y_legend, :y2_legend, :x_legend, :values, :values_2, :links, :x_labels, :y_max def initialize(title, actions_running_time, key, today) @title = title @actions_running_time = actions_running_time @key = key @today = today end def compute # convert to array and fill in non-existing weeks with 0 @max_weeks = difference_in_weeks(@today, @actions_running_time.last.created_at) @actions_running_per_week_array = convert_to_weeks_from_today_array(@actions_running_time, @max_weeks+1, :created_at) # cut off chart at 52 weeks = one year @count = [52, @max_weeks].min # convert to new array to hold max @cut_off elems + 1 for sum of actions after @cut_off @actions_running_time_array = cut_off_array_with_sum(@actions_running_per_week_array, @count) @max_actions = @actions_running_time_array.max # get percentage done cumulative @cumulative_percent_done = convert_to_cumulative_array(@actions_running_time_array, @actions_running_time.count ) @url_labels = Array.new(@count){ |i| url_for(:controller => 'stats', :action => 'show_selected_actions_from_chart', :index => i, :id=> @key, :only_path => true) } @url_labels[@count]=url_for(:controller => 'stats', :action => 'show_selected_actions_from_chart', :index => @count, :id=> "#{@key}_end", :only_path => true) @time_labels = Array.new(@count){ |i| "#{i}-#{i+1}" } @time_labels[0] = "< 1" @time_labels[@count] = "> #{@count}" # Normalize all the variable names @y_legend = I18n.t('stats.running_time_legend.actions') @y2_legend = I18n.t('stats.running_time_legend.percentage') @x_legend = I18n.t('stats.running_time_legend.weeks') @values = @actions_running_time_array.join(",") @links = @url_labels.join(",") @values_2 = @cumulative_percent_done.join(",") @x_labels = @time_labels.join(",") # add one to @max for people who have no actions completed yet. # OpenFlashChart cannot handle y_max=0 @y_max = 1+@max_actions+@max_actions/10 end # assumes date1 > date2 def difference_in_weeks(date1, date2) return difference_in_days(date1, date2) / 7 end # assumes date1 > date2 def difference_in_days(date1, date2) return ((date1.utc.at_midnight-date2.utc.at_midnight)/SECONDS_PER_DAY).to_i end def convert_to_weeks_from_today_array(records, array_size, date_method_on_todo) return convert_to_array(records, array_size) { |r| [difference_in_weeks(@today, r.send(date_method_on_todo))]} end # uses the supplied block to determine array of indexes in hash # the block should return an array of indexes each is added to the hash and summed def convert_to_array(records, upper_bound) a = Array.new(upper_bound, 0) records.each { |r| (yield r).each { |i| a[i] += 1 if a[i] } } a end # returns a new array containing all elems of array up to cut_off and # adds the sum of the rest of array to the last elem def cut_off_array_with_sum(array, cut_off) # +1 to hold sum of rest a = Array.new(cut_off+1){|i| array[i]||0} # add rest of array to last elem a[cut_off] += array.inject(:+) - a.inject(:+) return a end def convert_to_cumulative_array(array, max) # calculate fractions a = Array.new(array.size){|i| array[i]*100.0/max} # make cumulative 1.upto(array.size-1){ |i| a[i] += a[i-1] } return a end end end
Страшно, не правда ли?
Переносить код из одного места в другое не сложно. С золотым мастер-тестом, утверждающим, что все хорошо, ничего не изменилось , это даже не особенно страшно, а просто утомительно. Но мы больше не можем просто лопать. Единственный путь вперед — дразнить код отдельно.
Чтобы сделать это, нам нужно понять это, и мы поймем это, дразня это.
Вот предварительный план: переименуйте материал и задайте много вопросов.
Что это? Что это представляет? Как выглядит структура данных? Как это используется? Почему мы заботимся? Почему небо голубое? Мы уже на месте?
Возьми это сверху
Настоящая суть проблемы начинается с compute
. Вот первая строка:
@max_weeks = difference_in_weeks(@today, @actions_running_time.last.created_at)
Единственное, что я могу с уверенностью сказать, что я понимаю, это @today
.
Этому предшествует комментарий, преобразование в массив и заполнение несуществующих недель 0 , но что это вообще означает? Конвертировать что в массив? Массив чего ?
Ну, @actions_running_time
, @actions_running_time
. Что бы это ни было.
Оказывается, что @actions_running_time
— это коллекция незаконченных TODO. Мы могли бы назвать это @unfinished_todos
, но неясно, имеет ли это смысл в этом контексте.
Бритье ненужного контекста
Трассировка кода показывает, что мы отправляем в TODO только create_at.
ТОДО являются ядром домена. Модель Todo
имеет длину 420 строк, в ней много данных и много связей. Но здесь нас это не волнует. Для этого кода не имеет значения, что TODO незакончены.
Честно говоря, это даже не имеет значения, что они TODO.
Другими словами, unfinished_todos
— это не имя, которое мы ищем, потому что ни незаконченное, ни todo не является значимым различием в этом случае. Мы просто заботимся о отметке времени.
В идеале мы не должны даже передавать объекты Active Record в Stats::RunningTimeData
.
Контроллеры могут сделать map(&:created_at)
перед отправкой данных в Stats::RunningTimeData
.
Внести изменения просто — без ошибок, без сюрпризов.
Происходит несколько приятных вещей. Мы можем забыть о сложности незаконченных задач . Меньше контекста, меньше понимания, и объект Stats становится легче рассуждать. Это также позволяет легко назвать коллекцию. Что это? timestamps
!
Дополнительным преимуществом является то, что теперь мы можем легко создавать простые автоматизированные тесты, которые непосредственно обрабатывают различные части объекта Stats.
def test_stuff today = Time.utc(2014, 1, 2, 3, 4, 5) timestamps = [ # ... ] stats = Stats::RunningTimeData.new("title", timestamps, "key", today) # assert_equal "whatever", stats.something end
Это не юнит тест. Это больше похоже на тест эфемерной характеристики, единственной целью которого является изучение некоторого результата.
Возьми это сверху (снова)
Мне все еще интересно, о чем @max_weeks
. Извлечение этого в метод позволяет нам проверять это с помощью теста, видя, что является возвращаемым значением.
Попытка нескольких различных значений показывает, что max_weeks
является подсчетом того, сколько недель у нас есть незаконченные задачи. Или, если хотите, это сколько недель самой старой незаконченной задачи.
Лучшее имя может быть total_weeks_of_data
.
def total_weeks_of_data difference_in_weeks(today, timestamps.last) end
Оглядываясь назад, можно сказать, что имя diff_in_weeks самоочевидно, но сам метод щекочет мое чувство пауков.
# assumes date1 > date2 def difference_in_weeks(date1, date2) return difference_in_days(date1, date2) / 7 end
Значение date1
соответствует today
date1
, а date2
— самая старая отметка времени. Это все метки времени, но они названы в честь дат.
Некоторое разумное переименование оставляет нас вместо « today
и следующего определения метода:
# assumes timestamp1 > timestamp2 def difference_in_weeks(timestamp1, timestamp2) difference_in_days(timestamp1, timestamp2) / 7 end
Но это еще не все.
timestamp1
, которое передается в качестве аргумента, всегда now
, что доступно всему классу. Нет причин передавать это в качестве аргумента.
def difference_in_weeks(timestamp) difference_in_days(now, timestamp) / 7 end
Это становится странным. Разница с одним аргументом не имеет смысла. Это звук хлопка одной руки.
Лучшее имя будет weeks_since(timestamp)
.
Кажется вероятным, что мы можем внести аналогичные изменения в difference_in_days()
.
# assumes date1 > date2 def difference_in_days(date1, date2) return ((date1.utc.at_midnight-date2.utc.at_midnight)/SECONDS_PER_DAY).to_i end
Да. at_midnight
Это странно рекурсивно. Переменные называются как даты, но на самом деле они представляют собой метки времени, но метод на самом деле заботится о датах.
В Ruby 1 в date + 1
стоит один день, тогда как 1 в timestamp + 1
стоит одну секунду.
Принуждение значений в initialize
и переименование их снова! избавляет от всех спорных моментов, значительно упрощая метод:
def days_since(date) (today - date).to_i end
Метод days_since
используется только в одном месте, поэтому мы можем также встроить его:
def weeks_since(date) (today - date).to_i / 7 end
В качестве бонуса SECONDS_PER_DAY
просто стал лишним.
Мы уже на месте?
Ну нет. Мы даже не смотрели на строку 2 метода compute
, но уже есть существенное улучшение.
Вот с чего мы начали:
class RunningTimeData # ... SECONDS_PER_DAY = 86400; def compute @max_weeks = difference_in_weeks(@today, @actions_running_time.last.created_at) # 35 more lines end # assumes date1 > date2 def difference_in_weeks(date1, date2) return difference_in_days(date1, date2) / 7 end # assumes date1 > date2 def difference_in_days(date1, date2) return ((date1.utc.at_midnight-date2.utc.at_midnight)/SECONDS_PER_DAY).to_i end # ... end
И после прохождения кроличьей норы до самого конца мы получили:
class RunningTimeData # ... def compute # 35 lines end def total_weeks_of_data weeks_since(dates.last) end def weeks_since(date) (today - date).to_i / 7 end # ... end
Методы рефакторинга удаляют параметры, а встроенный метод помогает избавиться от путаницы и путаницы, позволяя появиться концепциям. Рефакторинг с использованием метода извлечения, метода переименования и переменной переименования закодировал открытые концепции в самой кодовой базе. (Примечание. Каталог методов рефакторинга можно найти здесь .)
Мы начали с грязной идеи, названной действиями во время выполнения . Мы упростили код, и появилась концепция временных меток . Как только туман Time
и Date
рассеялся, центральным понятием стали просто даты .
Это упрощение позволило нам легко создавать тесты эфемерной характеризации, мощный инструмент для исследования значения там, где он был скрыт из-за сложности.
До сих пор нет реального смысла в том, что такое Stats::RunningTimeData
. Для этого нам нужно будет пройти еще несколько кроличьих троп, которые приведут к открытию новых концепций. Оставайтесь с нами для следующего выпуска, где ясность возрастает как на дрожжах.