How to avoid code duplication when validating virtual attributes?

I recently had to do some work populating a model from complex virtual
attributes. However, I also wanted to be able to leverage the
validations framework to be able to add descriptions of errors that
existed in virtual attribute values. I’m wondering how to avoid
duplication between the code that transforms the virtual attributes
into real attributes, and the code that does the validation.

This is probably best demonstrated with a simplified example.

Before I start, it’s worth noting that this situation is a little
unusual in that it uses a model that isn’t actually created by the
user via a form. Instead, I have a Ruby process that monitors a POP3
server and, whenever a new email is received, parses the mail to
determine the sender from the email address, and saves it to the
database via a ‘Message’ model. If any errors occur saving the
message, it writes the errors to the log. I won’t go into the details
of checking the POP3 server, but suffice to say that I’d like the code
that actually processes each email to look something like this:

def receive(mail)
message = Message.new(:mail => mail)
unless message.save
// Write out the message validation errors to the log.
end
end

To support this, the Message model includes a ‘mail’ virtual
attribute. Assuming that this attribute is a TMail::Mail object, the
Message model looks something like this:

class Message < ActiveRecord::Base
belongs_to :sender

def mail=(mail)
@mail = mail
self.sender = Sender.find_by_email_address(mail.from_addrs[0]) if
mail.from_addrs.size == 1
end

def validate
from_addresses = @mail.from_addrs

if from_addresses.size == 1
  errors.add("'From' address", "is not a known sender ") unless

Sender.find_by_email_address(from_addresses[0])
else
errors.add(“‘From’ addresses”, “expected to contain one email
address”)
end
end
end

My concern is the duplicated logic for checking whether there’s just
one email address. It mightn’t look too bad now, but rapidly becomes a
pain when many other parts of the email have to be parsed and
validated (as is the case in real life). It’s also worth keeping in
mind that I have to double up my testing too.

I thought about just raising an exception if there were any problems
when setting the virtual attribute. However, this brought it’s own
complexities - to do it properly I have to define a custom exception
class, and get the caller to check if it has been raised.

I also thought about adding the errors in the virtual attribute setter
instead of the validate() method, but I’ve never seen this done before
and am not sure what side-effects it might have.

Any ideas?

Thanks,

Ben

You received this message because you are subscribed to the Google
Groups “Ruby on Rails: Talk” group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.

Ben T. wrote:

if from_addresses.size == 1
  errors.add("'From' address", "is not a known sender ") unless

Sender.find_by_email_address(from_addresses[0])
else
errors.add("‘From’ addresses", “expected to contain one email
address”)
end

I think your main problem is that the if statement is actually running
two validation actions and not just one.

So a first change could be to:

unless from_addresses.size == 1
errors.add("‘From’ addresses", “expected to contain one email
address”)
end

unless Sender.find_by_email_address(from_addresses[0])
errors.add("‘From’ address", "is not a known sender ")
end

That would simplify this a littel.

Also your code will be a lot easier to read and test if you stripped
each validation out into its own private method. So I’d rewrite your
validation method as:

def validate
@from_addresses = @mail.from_addrs
validate_only_one_from_address
validate_from_address_is_known_sender
end

private
def validate_only_one_from_address
unless @from_addresses.size == 1
errors.add("‘From’ addresses", “expected to contain one email
address”)
end
end

def validate_from_address_is_known_sender
unless Sender.find_by_email_address(@from_addresses[0])
errors.add("‘From’ address", "is not a known sender ")
end
end