« Getting rid of switch statements with Java Enums
An idea for a ruby inspection tool »

Granularity of Abstractions

In Java, iterating over a collection and collecting some objects based on a condition is a classic example of something to put into a method. Here is an example:

  1. private Collection<Claim> getExpiredClaims() {
  2.     List<Claim> expiredClaims = new ArrayList<Claim>();
  3.     for (Claim claim : allClaims) {
  4.         if (claim.isExpired()) {
  5.             expiredClaims.add(claim);
  6.         }
  7.     }
  8.     return expiredClaims;
  9. }

In Ruby, you can do this in one line.

  1. all_claims.select { |claim| claim.expired? }

It’s lovely how much more succinct that operation becomes. It can actually get more succinct by using Symbol#to_proc:

  1. all_claims.select (&:expired?)

There is still the question of whether to move this to a method. Now, moving it to a method doesn’t reduce multi-line duplication the way it would in the Java example; there is only a minor amount of single line duplication that can be reduced.

The resistance I often get to refactoring a single line to a method is that it creates more total lines in the file with the def/end lines and spacing. Also, it takes a slight amount of effort to do the refactoring, and some developers won’t refactor unless the need is glaring and obvious.

There are still important reasons to move this to a method, and those reasons are better readability of code and reducing duplication of expressions. Without the refactoring, you’ll see code that looks like this:

  1. def mark_expired_claims_for_review
  2.     all_claims.select (&:expired?).each(&:needs_review!)
  3. end
  4.  
  5. def notify_claim_agents_of_expired_claims
  6.     all_claims.select (&:expired?).each(&:notify_agent_of_expiration)
  7. end

After the refactoring, this is what it would look like.

  1. def expired_claims
  2.     all_claims.select(&:expired?)
  3. end
  4.  
  5. def mark_expired_claims_for_review
  6.     expired_claims.each(&:needs_review!)
  7. end
  8.  
  9. def notify_claim_agents_of_expired_claims
  10.     expired_claims.each(&:notify_agent_of_expiration)
  11. end

It’s a minor point perhaps, and I’m sure that I could come up with much more excessive examples, but I just wanted to focus on a simple example. By replacing the expression with a symbolic reference, a method in this case, you express in English something that is a programmatic operation. This improves readability quite a bit.

Also, there is the additional benefit of abstracting on the concept of expired claims. By having multiple places using “all_claims.select(&:expired?)” to express expired claims, you duplicate the implementation detail that expired claims are derived from a larger collection of claims. This may not always be true, and a change in that derivation results in a change in many places.

Perhaps the question here is how DRY should you make your code. I’m still undecided on this on this point, but I think that the amount of work that it takes to reduce duplication to this level is minimal, and the result will be an important part of a pristine codebase.

Tags: , ,

This entry was posted on Thursday, October 23rd, 2008 at 8:24 pm and is filed under The Art of Programming. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

One Response to “Granularity of Abstractions”

  1. C# new language features: Conciseness v Readability at Mark Needham Says:
    November 24th, 2008 at 7:45 am

    […] One idea I am considering trying is using methods which describe more clearly what the lambda function is doing. This is an idea I came across from Kris Kemper’s post about using similar Ruby language features. […]

Leave a Reply