# How to avoid code duplication when validating virtual attributes?

**URL:** <https://www.ruby-forum.com/t/how-to-avoid-code-duplication-when-validating-virtual-attributes/181028>\
**Category:** Rails\
**Created:** [January 4, 2010, 7:14am UTC](https://www.ruby-forum.com/t/how-to-avoid-code-duplication-when-validating-virtual-attributes/181028 "2010-01-04T07:14:25Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![nbits](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/n/8c91f0/32.png) [@nbits](https://www.ruby-forum.com/u/nbits)\
**Post date:** [January 4, 2010, 7:14am UTC](https://www.ruby-forum.com/t/how-to-avoid-code-duplication-when-validating-virtual-attributes/181028/1 "2010-01-04T07:14:25Z")

</div>

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 removed\_email\_address@domain.invalid.  
To unsubscribe from this group, send email to  
removed\_email\_address@domain.invalid.  
For more options, visit this group at  
[http://groups.google.com/group/rubyonrails-talk?hl=en](http://groups.google.com/group/rubyonrails-talk?hl=en).

---

<div class="post-metadata">

**Author:** ![nbits](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/n/8c91f0/32.png) [@nbits](https://www.ruby-forum.com/u/nbits)\
**Post date:** [January 4, 2010, 2:09pm UTC](https://www.ruby-forum.com/t/how-to-avoid-code-duplication-when-validating-virtual-attributes/181028/2 "2010-01-04T14:09:22Z")

</div>

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