Ruby Rspec Timer — решение для рефакторинга

Я решил вопрос TestFirst.org 09_timer для тестирования Ruby Rspec. Мой код работает, но мне это не нравится. Это очень долго. Буду признателен за любые комментарии и/или предложения по его улучшению. Пожалуйста, включите объяснение, чтобы прояснить любые предложения. Цель состояла в том, чтобы создать таймер с переменной экземпляра @seconds, инициализированной до 0, а затем вернуть все значения в виде строки с часами, минутами, секундами в формате: 00:00:00. Итак, 12 секунд => 00:00:12; 66 секунд => 00:01:06; и 4000 секунд => 01:06:40. Спасибо. Код ниже.

class Timer

    attr_accessor :seconds

    def initialize
      @seconds = 0
    end

    def padded(n)
      "0#{n}"
    end

    def time_string
      hours = @seconds/3600
      h_minutes = ((@seconds%3600)/60)
      minutes = @seconds/60
      m_seconds = @seconds%60
      second = @seconds
      seconds = ""

      if @seconds < 60
        if second < 10
          second =  padded(second)
        end
        seconds << "00:00:#{second}"
      elsif @seconds > 3600
        if hours < 10
          hours = padded(hours)
        end
        if h_minutes < 10
          h_minutes = padded(h_minutes)
        end
        if m_seconds < 10
          m_seconds = padded(m_seconds)
        end
        seconds << "#{hours}:#{h_minutes}:#{m_seconds}"
      else
        if minutes < 10
          minutes = padded(minutes)
        end
        if m_seconds < 10
          m_seconds = padded(m_seconds)
        end
        seconds << "00:#{minutes}:#{m_seconds}"
      end
      @seconds = seconds    
    end

end

person Jeremiah McCurdy    schedule 27.01.2014    source источник


Ответы (1)


Есть несколько небольших вещей, которые вы можете сделать, чтобы упростить свой класс, и несколько крупных организационных изменений.

1) Используйте String#rjust для заполнения цифр:

def padded(n)
  "#{n}".rjust(2, '0')
end

Это позволяет применять его к каждому числу, независимо от того, состоит ли оно уже из двух цифр. Как следствие, вы можете избавиться от всех однозначных проверок (if h_minutes < 10 и т. д.).

2) Избавьтесь от всего, начиная с первого оператора if, так как ничего из этого не нужно. Всего несколькими строками ранее у вас есть hours = @seconds / 3600, h_minutes = ((@seconds%3600)/60) и m_seconds = @seconds%60, и это единственные три значения, которые вам нужны. Примените простую карту (для заполнения) и соедините с ":", чтобы получить окончательную строку.

3) Если вам нужен объектно-ориентированный подход, каждая из ваших переменных часов/минут/секунд может быть методом, поэтому вы получите что-то вроде этого:

class Timer
  attr_accessor :seconds
  def initialize
    @seconds = 0
  end

  def time_string
    [hours, minutes, m_seconds].map(&method(:padded)).join(":")
  end

  def hours
    seconds / 3600
  end

  def minutes
    (seconds % 3600)/60
  end

  def m_seconds
    (seconds % 60)
  end

  def padded(n)
    "#{n}".rjust(2, '0')
  end
end
person Zach Kemp    schedule 27.01.2014
comment
Большое спасибо, Зак. Вы только что продвинули мое понимание на несколько недель, вероятно, одним простым решением. Одна из моих самых больших проблем была с часто повторяющимися одиночными проверками, которые вы выполняли с помощью простого метода #rjust. Я никогда даже не рассматривал это. И я никогда не использовал #map таким образом, но благодаря вам я теперь знаю о методе Symbol#to_proc. Большие пальцы вверх. - person Jeremiah McCurdy; 28.01.2014
comment
Рад слышать, Джере.мак. rjust — это одна из тех вещей в стандартной библиотеке, которая встречается редко, но полезно знать, когда она вам нужна. Вы обнаружите много подобных вещей по мере того, как будете ближе знакомиться с языком. - person Zach Kemp; 28.01.2014