{"id":759,"date":"2011-06-11T15:56:00","date_gmt":"2011-06-11T15:56:00","guid":{"rendered":"https:\/\/www.keenertech.com\/?p=759"},"modified":"2011-06-11T15:56:00","modified_gmt":"2011-06-11T15:56:00","slug":"the-empty-string-code-smell-in-rails","status":"publish","type":"post","link":"https:\/\/staging.keenertech.com\/?p=759","title":{"rendered":"The Empty String Code Smell in Rails"},"content":{"rendered":"\n

Sometimes you look at some source code and it just doesn’t look right. The code might seem inordinately complex for the simple task being accomplished. Or the developer might be performing certain actions repeatedly, such as checking variables for nil values. This is what is known as a code smell.<\/em>.<\/p>\n\n\n\n

Code smells should be corrected at the earliest possible opportunity. In this article, I’ll talk about some real-world coding issues that my friend, Robb Kidd, and I recently encountered.<\/p>\n\n\n\n

The Repetitive Value-Checking Code Smell<\/h3>\n\n\n\n

Here’s a code smell that I found in a Rails view:<\/p>\n\n\n\n

   <% if @item.summary.nil? || @item.summary == ” %>
      <p>Not Defined<\/p>
   <% else %>
      <p><%= @item.summary %><\/p>
   <% end %><\/p>\n\n\n\n

This type of code was provided for every field associated with an “item.” Not good. Not surprising, though. This is a pretty common code smell from Ruby newbies.<\/p>\n\n\n\n

Here’s a slightly better modification:<\/p>\n\n\n\n

   <% if @item.summary.blank? %>
      <p>Not Defined<\/p>
   <% else %>
      <p><%= @item.summary %><\/p>
   <% end %><\/p>\n\n\n\n

The blank?<\/em> method checks for both nil and empty strings in one method. It even works on arrays:<\/p>\n\n\n\n

   arr = []
   arr.blank?<\/p>\n\n\n\n

If the array is empty, as it is in the code above, then blank?<\/em> returns true.<\/p>\n\n\n\n

The blank?<\/em> method is awfully convenient for these types of checks. It also has a companion method, present?<\/em>, that does the inverse check, i.e. \u2014 it checks if a value is present (ensuring that the value is not blank).<\/p>\n\n\n\n

We can shorten the original code even further:<\/p>\n\n\n\n

   <p><%= @item.summary.blank? ? ‘Not Defined’ : @item.summary %><\/p><\/p>\n\n\n\n

Now we’ve got the original code sample down to one line using what amounts to an inline if-statement. That’s pretty good.<\/p>\n\n\n\n

But we’re still repeating that if-statement for every field displayed. To DRY the code up even further, we could extract that statement into a helper method:<\/p>\n\n\n\n

   def format_text(txt)
      txt.blank? ? ‘Not Defined’ : txt
   end<\/p>\n\n\n\n

In the view, the code then becomes:<\/p>\n\n\n\n

   <%= format_text(@item.summary) %><\/p>\n\n\n\n

Now the view is simple. Just the way I like it.<\/p>\n\n\n\n

The Empty String Code Smell<\/h3>\n\n\n\n

I still wasn’t happy though. You see, the developer had told me that the reason he needed to check for empty strings everywhere was because empty strings were being physically stored in the database.<\/p>\n\n\n\n

This was something that Robb Kidd and I began investigating immediately.<\/p>\n\n\n\n

To me, this code smell was far worse than the original view-related one that had triggered this deep dive into the codebase. I was appalled that empty strings were being stored in the database.<\/p>\n\n\n\n

This might not seem like a big deal to many Rails developers. But with the types of high-end enterprise systems that I typically build, high-quality data is vital. Relational databases represent empty fields as NULL. Once you start putting empty strings into database fields, you have compromised the usefulness of NULL to represent the absence of information. You’ve also doomed everybody who ever works with that database to check for both NULL’s and empty strings if they want to determine whether a valid value is present.<\/p>\n\n\n\n

You’ve stored a code smell in the database for all eternity. Not good.<\/p>\n\n\n\n

Even worse, the database may be an underlying component of a Rails application, but there’s nothing in an enterprise setting that ties it exclusively to the Rails application. If the Rails application is successful, then much of the application’s value derives from the integrity of the data that has been stored. Other applications, sometimes implemented in other languages, may be built on top of that database.<\/p>\n\n\n\n

By pushing a code smell into the database, we’re forcing all future developers, not just Rails developers, to deal with the compromised usefulness of NULL. We may very well hear this from some future Python developer, “Jeez. Those Rails dweebs were amateurs. This wouldn’t have happened if they’d used Django.”<\/p>\n\n\n\n

We should definitely deal with this problem. But first, how does it happen?<\/p>\n\n\n\n

The application allows users to enter data via forms. Upon submission, Rails collects all submitted values into the standard params<\/em> hash that is made available to controllers. Any form field that is a text field or text area will be assigned the empty string as a value. the following controller code perpetuates the empty string into the database:<\/p>\n\n\n\n

   Item.create!(params[‘item’])<\/p>\n\n\n\n

Whoa. So I can hear you asking: “So this is a Rails problem?”<\/p>\n\n\n\n

Yes. It is a Rails problem, and one that in my humble opinion should never have slipped through the cracks.<\/p>\n\n\n\n

But we still need to correct it in our application. So, Robb and I started researching possible solutions.<\/p>\n\n\n\n

The best solution we came across was from Henrik Nyh<\/a>, a Ruby developer in Stockholm, Sweden. He published an excellent code snippet<\/a> on GitHub’s gist sub-site. This is a site at gist.github.com<\/a> that allows developers to publish short snippets of code that solve problems.<\/p>\n\n\n\n

His code is a mix-in module that updates ActiveRecord’s write_attribute method to properly handle empty strings.<\/p>\n\n\n\n

   module NullifyBlankAttributes

      def write_attribute(attr_name, value)
         new_value = value.presence
         super(attr_name, new_value)
      end

   end<\/p>\n\n\n\n

Note the use of the presence<\/em> method, which takes an argument and returns either the argument itself or nil if the argument was blank.<\/p>\n\n\n\n

Now what? What do we do with the module?<\/p>\n\n\n\n

First, drop the module in the lib<\/em> directory of the Rails application. Second, create an initialization file in the config\/initializers<\/em> directory of the Rails application. Let’s call it active_record_fixes.rb<\/em>. The contents of the file will be:<\/p>\n\n\n\n

   class ActiveRecord::Base
      include NullifyBlankAttributes
   end<\/p>\n\n\n\n

We have just monkey-patched Rails to correct our empty string code smell.<\/p>\n\n\n\n

Conclusions<\/h3>\n\n\n\n

We discovered a code smell in our view code that masked a more pernicous underlying code smell. We got rid of both code smells with the help of a developer from Sweden that we’ve never met.<\/p>\n\n\n\n

There are a few lessons to be learned from all this. First, code smells increase the maintenance profile of your application over time, making it more difficult to add new features and maintain existing ones. Code smells are an indication of a coding problem, and should be fixed as soon as feasible, even if the code is functionally working just fine.<\/p>\n\n\n\n

Second, when you have a problem, don’t re-invent the wheel. We’re an international coding community. Somebody may very well have solved the problem already. Spend some time researching using Google in case somebody’s already developed a solution. In this case, Henrik solved the problem nicely a few years ago.<\/p>\n\n\n\n

Finally, give back to the community. If you solve a problem that might reasonably impact others in the community, post a solution. Write a blog entry (as I have done). Or post a code snippet on gist.github.com<\/em>, or answer a question posed on sites like Stack Overflow<\/a>. Coding is hard enough as it is. Let’s share some solutions to make it easier on all of us.<\/p>\n\n\n\n

Reception<\/h3>\n\n\n\n

This article was well received and generated a number of excellent comments. Since I don’t have a way of porting them in smoothly from the old website, I’ve included snapshots of the comments…<\/p>\n\n\n\n

\"First<\/figure>\n\n\n\n
\"Second<\/figure>\n\n\n\n
\"Third<\/figure>\n\n\n\n
\"Fourth<\/figure>\n","protected":false},"excerpt":{"rendered":"

Sometimes you look at some source code and it just doesn’t look right. The code might seem inordinately<\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[8],"tags":[160,172],"class_list":["post-759","post","type-post","status-publish","format-standard","hentry","category-technology","tag-rails","tag-ruby"],"_links":{"self":[{"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=\/wp\/v2\/posts\/759","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=759"}],"version-history":[{"count":0,"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=\/wp\/v2\/posts\/759\/revisions"}],"wp:attachment":[{"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=759"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=759"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/staging.keenertech.com\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=759"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}