Writing good code

Maricris Nonato -

It has long been a debate on how to describe good code. What are the criteria for declaring a code to be good, neat, and dry?

My advocacy is to promote writing good quality code. It is by painstaking practice and tons of reading that I get to exercise this advocacy. It is but a long journey, but little steps lead you to understanding and qualifying a code to be of high quality. So what are my benchmarks?

  • Long codes are less readable and more likely to fail.

Ruby scripts are far smaller and more compact in size compared to Java codes or C++ codes. Trust me, I''ve seen some and comparing them to Ruby, reading Java codes or C++ codes that are not mine would take me a longer time for understanding (even if docs and other help materials are present). Ruby has good metaprogamming (that sometimes gets abused because of monkey patching) and produce straightforward codes. Each line would end up more like real English sentences.

If you write ruby codes that take up to more than twenty lines.. hmm.. well, its doomed to fail, for sure. :) Refactor and review!

  • Codes that don't look anything like its API source is really smelly.

APIs are there for you to use as a general sitemap. It is your guide to efficiently write code with ease. Usually, APIs are well documented and even provide examples or snippets of codes to demonstrate its use. The lines of codes involved may also be present for you to inspect its process and even predict if it will pass your expectations.

If you write your own method wanted to create something similar to (say link_to), then you must also follow the existing format of the method itself. Don't write methods that look or will work like something that's already existing in the API wherein the final script will look nothing like its ancestor method.

  • Repeatedly using a block across the whole application is not DRY, its wrong

Dubious codes are the best candidates for DRYing. Codes that appear across the application and across a method/controller are codes that should be rewritten. Exercise DRY. Always see what you can refactor. As much as possible, revisit existing methods or parts of the site that you suspect could be doing the same thing as your end goal.

Once you'll find a section/method whose output is "quite" similar to your expected result, nominate this method to a higher level and extend it. Don't create another method separate from this and create clutter. This is one road towards failure.

  • Stop reinventing the wheel.

Sure, you're great. No one is better than you. Well, you're wrong.. and chances are, if you think this way.. you're dead stuck at the bottom of the career ladder!

A responsible, mature programmer would try and see first if that something very trivial has already been done by somebody else. Research and extend what is already there. Be responsible and diligent enough to see if there is a better, faster and more elegant solution to your problem. Don't ditch the time for planning and go about writing your own code from scratch!

If you work like nothing has already been done by your ancestors or your entire community, then you're still a student. Stop writing collegiate machine problems, and get down to creating quality applications.

In line with this, I'd like to pinpoint an example. I found this code somewhere in one of our partials. At first, it wouldn't have caught my eye.. until I found this method called cut_string. Its worth noting that we were using Rails version 1.2.6 and that not every bit of beauty found in Rails version 2.1.0 is already there. (This is one of the burdens of writing your app early, and delaying upgrades to the very last end.)

From its name cut_string, I immediately was reminded of the truncate method native in Rails. The method was written in our application_helper. I felt the need to read and understand this uncommented code and see if its anything different from truncate. To my surprise, it wasn't!

def cut_string(original, num_of_chars)
 truncated = original
 
 if original != nil
   if num_of_chars >= original.length
     max = original.length
   else
     max = num_of_chars
   end
 
   truncated = original[0,max]
   space_pos = truncated.rindex(' ')
   if space_pos !=nil && space_pos != max
     truncated = truncated[0,space_pos.to_i]
   end
  end

  truncated = strip_html(truncated) if truncated
  return truncated
 end

Blindly and irresponsibly rewriting it, I would've:

def cut_string(original, num_of_chars)
  unless original.nil?
    max = (num_of_chars >= original.length) ? original.length ? num_of_chars
    truncated = original[0,max]
    space_pos = truncated.rindex(' ')
    truncated = strip_html(truncated[0,space_pos.to_id]) if (!space_pos.nil? and space_pos!=max)
  end
  original
end

After this irresponsible rewrite, I would've saved 8 lines of code. Now what were my assumptions in this rewrite?

  • I only need this operation if original string is not blank (then I have saved some computing power)
  • I saved a whole block of "if..then..else" by using ternary operators (thereby reducing it to a one liner) since I only have to do value assignment
  • I made use of the fact that "the last assignment or code output of the last line in a method is always the return value of Ruby"

If you're the "easily satisfied programmer", you'd say that this code is already okay. But wait! There's more. ;) I am not lazy, and by experience, I know for a fact that I can still reduce this 7 liner block into a mere 2 liner block! Yep, I'm not kidding.

Rails already have this truncate method that I can make use of. I fire up my ROR API (found online at http://api.rubyonrails.org) and check the usage of truncate method and I find this:

truncate(text, length = 30, truncate_string = "...")      
  truncate("Once upon a time in a world far far away", 14)
  => Once upon a...
  [ hide source ]
  # File actionpack/lib/action_view/helpers/text_helper.rb, line 34
  34: def truncate(text, length = 30, truncate_string = "...")
  35: if text.nil? then return end
  36: l = length - truncate_string.chars.length
  37: text.chars.length > length ? text.chars[0...l] + truncate_string : text
  38: end

I read carefully and understood that what cut_string wanted to do was eliminate the cases wherein the sentence or string would be forcibly cut into words that are not dictionary-valid. This was the whole reason of wanting to get space_pos = truncated.rindex(' ') into the block. I didn't want to lose this essence, so I made this new method that will make use of my knowledge of Rails and Ruby programming. I know have this:

def truncate_string(text, length, truncate_string = "...")
  str = truncate(text, length)
  str[0..str.rindex('' '')] + truncate_string
end
  • I was able to follow Rails' method declaration for "truncate"
  • I was able to truncate the string into the desired (maximum) number of characters
  • I was able to retain the essence of words not being cut into unreadable chunks
  • I was able to maintain the dynamic use of whatever truncate_string would be preferred
  • I was able to reduce code by ~86%
  • I was able to enjoy it!

Got it? I hope you did. Happy coding!