Old Guy New Trick

An old guys journey to learn how to code.

Clean up your code


Author: John on November 24, 2015

Today I'd like to share two examples of my initial code, and then how I refactored that code.  The first example I worked out on my own, and the second was hinted to me by my co-worker/mentor.

We needed a new method that would take a collection of bidders, and possibly a credit (discount) type.  The credit type would either be "" (blank,) small, medium or large.  For each of those credit types, we want a total count. Below is my first crack at a solution:

def bidders_total_by_type(bidders, credit_type = nil)
  count = 0
  bidders.each do |bidder|
    count += 1 if bidder.bidding_credit_type == credit_type
  end
  count
end

While the above worked fine, it just didn't sit well with me.  I thought, surely, there is a better way.  And I'm always trying to improve on my coding skills.  After a little research of the Ruby API, I found a better way to accomplish the same thing:

bidders.count { |bidder| bidder.bidding_credit_type == credit_type }

For my second example, I cannot take full credit.  While what I had below worked fine, my co-worker/mentor felt that I could improve my code.  He didn't tell me what to do, just that it could be better and a nudge in the right direction. Here is my original code:

def format_zero_dollar(value)
  return "$#{value}" if value == 0
  value
end

And now, the improved solution.  Since we will always return $0 if the passed in value is equal to zero, there is no reason to use string interpolation.  Here is a better way to accomplish the same thing:

def format_zero_dollar(value)
  return "$0" if value == 0
  value
end

Learn Something New Everyday!

Last Edited by: John on November 29, 2015