Method Composition

I figured I'd continue my thoughts on writing readable code and post about the advantages of method composition. I practice this pattern religiously as I think it adds an incredible amount of readability to my code. It is another fairly common technique and one that is quite easy to implement. Have a peek at the following example, http://pastie.org/473152 (Had to use pastie it's kind of long.)

Now that is pretty readable. The method body is not too large but there is a lot happening there. Take a look at a quick refactor (about 15 minutes, thanks specs) and let me know if you like the changes:

def self.render_timeline(slice)
data, data2, total_users = collect_data_from_slice(slice)
data_points = data.collect{|m| m[1] }
max = data_points.max
increment_max_until_last_set_of_ten_is_reached(max)
get_second_set_of_datapoints_if_slice_using_second(slice, datapoints, max)
graph_for_breadcrumbs= new_graph_with_data_points(slice, datapoints, max)
setup_right_axis_if_slice_using_second(slice, datapoints, max, graph_for_breadcrumbs)
colorize_graph(graph_for_breadcrumbs)
graph_for_breadcrumbs.set_labels_for_x_and_y_axis(slice, data)
graph_for_breadcrumbs.render
end

Do you see the advantages in readability & maintability? Granted I would bury this code in some sort of class for rendering the graph and perhaps a separate module for extracting data from the slice but you can see how much easier it is to gather what's going on with this method body, right? All I did was take code that was bulky or by itself didn't have an obvious meaning and wrapped it in a method body that gave it purpose. The other thing we did here is clearly expressed the intent of the render_timeline (renamed from timeline) method. As you read, you can almost predict what will happen next (http://en.wikipedia.org/wiki/Principle_of_least_astonishment.) This is great because we now have code that is readable, yields expected behavior, and can be reused by other members on the team!

There are some other great points here that I did not delve into: how easy a rework like this is with specs, composition vs inheritance, etc. But I'll save those for another time :)

Quick note, there are a couple gems I like to use to identify problem code. I especially like to use these gems when I am refactoring large methods/classes:

Roodi, http://github.com/martinjandrews/roodi/tree/master
Flog, http://ruby.sadi.st/Flog.html

Happy Coding!

Magic Numbers

I have been dabbling in 4 or 5 different code bases over the last couple weeks and over the course of these weeks I have noticed a much too common pattern involving magic numbers. Every few files I would come across a number that was part of a class definition, method call, etc that had no significant meaning by itself. Take a quick peek at this line of code I found & refactored inside of a class definition:

def get_remaining_schedules
15 - schedules.size
end

Now at second glance you can probably gather that the maximum # of schedules is 15 and we are just subtracting the current count from that total to get the remainder available. However, wouldn't this make more sense at a first glance if we did something like this?

MAXIMUM_SCHEDULES_ALLOWED=15

def get_remaining_schedules
MAXIMUM_SCHEDULES_ALLOWED - schedules.size
end

Now you can immediately see what is happening here as you skim the code (I like to use constants in a case like this because they stand out to my eye.) Now you may say that makes sense but within the method body the # 15 is understandable anyways? Why go the extra step? Well, I think it's important for people to be able to see exactly what the programmer's intent was at a glance if possible. So I say why not? What happens if we end up needing to know the maximum schedules allowed somewhere else in the class or calling code? Do we just drop the # 15 wherever we need to know the maximum schedules allowed? You can see how that might spiral in a hurry especially when your co worker has to grep for the #15 and try to figure out if it is related to what he/she is doing or not! Take a peek at another piece of this code I modified:

#snippet I refactored
<%= link_to_unless (15-@user.schedules.size)==0, "New Schedule", new_schedule_path %>

#refactor
<%= link_to_unless @user.schedules_limit_exceeded?, "New Schedule", new_schedule_path %>

#Which one is more readable? Which one is easier to maintain?

I think it's obvious sometimes when you're looking at code from an outside perspective or reviewing it after the fact but that is also why I dropped this post. I think any programmer that is refactoring and reviewing their work consistently would see that magic numbers take away from readability, add to maintenance headaches, and ultimately make sure not to use them.

In any event, this is just a quick little tidbit you can use when you brush by some code and have to stop, say "What the hell is that?", then answer yourself "Oh I see.". This is one of the simplest things you can quickly refactor into something much more readable at little cost to you or your employer.

In closing, happy coding and remember readability and maintainability is what we are striving for so the next time you see your co worker they say "Thanks for that sweet code bro!".