Raising errors in attribute accessor overrides?

Question

I am overriding an attribute accessor in ActiveRecord to convert a string in the format "hh:mm:ss" into seconds. Here is my code:

class Call < ActiveRecord::Base
  attr_accessible :duration

  def duration=(val)
    begin
      result = val.to_s.split(/:/)
             .map { |t| Integer(t) }
             .reverse
             .zip([60**0, 60**1, 60**2])
             .map { |i,j| i*j }
             .inject(:+)
    rescue ArgumentError
      #TODO: How can I correctly report this error?
      errors.add(:duration, "Duration #{val} is not valid.")
    end
    write_attribute(:duration, result)
  end

  validates :duration, :presence => true,
                       :numericality => { :greater_than_or_equal_to => 0 }

  validate :duration_string_valid

  def duration_string_valid
    if !duration.is_valid? and duration_before_type_cast
      errors.add(:duration, "Duration #{duration_before_type_cast} is not valid.")
    end
  end
end

I am trying to meaningfully report on this error during validation. The first two ideas that I have had are included in the code sample.

1. Adding to errors inside of the accessor override - works but I am not certain if it is a nice solution.
2. Using the validation method duration_string_valid. Check if the other validations failed and report on duration_before_type_cast. In this scenario duration.is_valid? is not a valid method and I am not certain how I can check that duration has passed the other validations.
3. I could set a instance variable inside of duration=(val) and report on it inside duration_string_valid.

I would love some feedback as to whether this is a good way to go about this operation, and how I could improve the error reporting.

Answer 1

First, clean up your code. Move string to duration converter to the service layer. Inside the lib/ directory create StringToDurationConverter:

# lib/string_to_duration_converter.rb
class StringToDurationConverter
  class << self
    def convert(value)
      value.to_s.split(/:/)
         .map { |t| Integer(t) }
         .reverse
         .zip([60**0, 60**1, 60**2])
         .map { |i,j| i*j }
         .inject(:+)
    end
  end
end

Second, add custom DurationValidator validator

# lib/duration_validator.rb
class DurationValidator < ActiveModel::EachValidator
  # implement the method called during validation
  def validate_each(record, attribute, value)
    begin
      StringToDurationConverter.convert(value)
    resque ArgumentError
      record.errors[attribute] << 'is not valid.'
    end
  end
end

And your model will be looking something like this:

class Call < ActiveRecord::Base
  attr_accessible :duration

  validates :duration, :presence => true,
                       :numericality => { :greater_than_or_equal_to => 0 },
                       :duration => true

  def duration=(value)
    result = StringToDurationConverter.convert(value)
    write_attribute(:duration, result)
  end
end