What was I thinking?

At my day job I am working on an API into an E911 interface and while writing my DSL to ingest and perform certain actions against their SOAP interface I quickly noticed an opportunity to refactor.  The end result is much cleaner and in my opinion easier to decipher what is happening.

To bring you up to speed this particular function is for searching against their database for a location, there are several “optional” fields you can send over to narrow and scope your search.  Obviously to properly expose the feature my DSL needs to be able to account for all the possible options.  Here is what the code was before refactoring

(WARNING: it is ugly)

message = { authentication: credentials }
# check for the various allowable options and build up the message
if options.key?("erlID")
  message["erlID"] = options["erlID"]
if options.key?("Addressdata")
  message["Addressdata"] = options["Addressdata"]
if options.key?("CustomCallback")
  message["CustomCallback"] = options["CustomCallback"]
if options.key?("Language")
  message["Language"] = options["Language"]
if options.key?("forceCsz")
  message["forceCsz"] = options["forceCsz"]
if options.key?("notificationOption")
  message["notificationOption"] = options["notificationOption"]
if options.key?("searchInSubAccounts")
  message["searchInSubAccounts"] = options["searchInSubAccounts"]

While very verbose, it smacks the DRY principle right in its face and it is flat out ugly.

This is what I did to refactor this little snippet of code

message = { authentication: credentials }
# check for the various allowable options and build up the message
["erlID", "Addressdata", "CustomCallback", "Language", "forceCsz", 
"notificationOption", "searchInSubAccounts"].each do | key |
  message[key] = options[key] if options.key?(key)

It takes less typing, less code, less eye-stress trying to follow along. Its quite DRY and you can ascertain what it does rather quickly.

NOTE: Yes I know it uses string as the key names instead of cleaner symbol key (made even better in Ruby 2.0), but because I have to put these in a specific format for the SOAP request (and I am using the very helpful Savon.rb gem) this was easier.

One has to question why I even allowed all those if/ends in the beginning anyhow. Perhaps I did not have enough coffee that day?!

How would you refactor that bit of code?

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>