Ben Teese
2010-Jan-04 06:12 UTC
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 rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
Rob Nichols
2010-Jan-04 13:09 UTC
Re: How to avoid code duplication when validating virtual attributes?
Ben Teese 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") > endI 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 -- Posted via http://www.ruby-forum.com/. -- 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 rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.