January 24, 2010
Older: Multiple Domain Page Caching
Newer: Correct, Beautiful, Fast (In That Order)
Just In Time, Not Just In Case
Something that has finally become a habit for me is adding code when it is needed, not in case it is needed. Often times we “think” we are going to need something, so we add the code to support it. What happens most often with that code is it sits and rots. It adds bloat and weight to your program that is not needed.
Example
I like to see examples first, so I will start with one. In Harmony, sites have users. I had a method in the site model named add_user.
class Site
def add_user(user)
user_id = user.is_a?(User) ? user.id : user
# add user code here, removed for clarity
end
end
Some might look at that and think it is a perfectly fine method, but I would disagree. A quick search through my code revealed that I was never passing a user id. Every time I called the method, I passed in a user as an argument. That was the first smell.
The second, which may be for another article is that I used is_a?
. We are programming in Ruby folks. It is always better to duck type and ask respond_to?
than is_a?
. The difference is asking an object what it is, instead of asking an object what it can do.
Having add_user
take either a user or a user id meant twice as many tests and an extra line of code. I killed the is_a?
ternary line and the accompanying tests and went on my merry way.
How to Avoid Just in Case
The best way that I have found to avoid “just in case” is to test first. When I write my tests first and start with the most basic test I can think of and go from there, my code tends to only be as complex as the test requires. Whenever I begin with program code instead of test code, I end up with “just in case” features like the one above.
This might seem small as it was only one line and it was well tested, but this was only one small example. Cases like this multiply as the program grows and the “real” code begins to get obfuscated behind the cases that might happen someday.
The moral of the story is a line removed is a line that will never have bugs down the road.
*To give credit where I believe credit is due, I am 99% positive that Jamis Buck said “Just in time, not just in case.” at a conference. Could not find a reference though.
10 Comments
Jan 24, 2010
Great example of one of the biggest benefits of TDD, which is YAGNI (You Ain’t Gonna Need It).
However, if you were to implement the original behavior, then I think using is_a? would be valid. I imagine that the method would only use User#id, but checking responds_to?(:id) would not be very robust. In this case, your intention would be to see if a User object was passed in, and is_a? would tell you that most clearly.
Jan 24, 2010
YES YES YES!
I can’t tell you how many times I go to make a small change (in my code and other’s) and there’s so much unnecessary flexibility.
Every time you make something flexible in one direction, you constrain it in another. And if my experience is any indication, you’re gonna want that OTHER direction long before the one you thought you’d need in the beginning.
Jan 24, 2010
I agree with your sentiment that special case code tends to obfuscate the ‘real’ code. I feel that way about performance ‘tweaks’ and caching on larger projects.
Though when you said you’re checking for ‘respond_to?’ I’d even think that may be code you don’t need, unless your users can do something that causes your application’s code to pass in an object that was unexpected.
If you’ve tested your interactions you wouldn’t even have to check things respond to methods, and duck type without checking anything (though then how do you communicate what that thing is or could be).
Oh the horror!
Jan 24, 2010
@Ryan Allen: The respond_to? wasn’t so much for this case as for others. I’ll probably post something more specific soon.
Jan 24, 2010
“My code tends to only be as complex as the test requires”…besides helping me divide and conquer, this is the biggest way that writing tests and/or doing TDD has improved the way I write code.
Jan 25, 2010
Excellent post about something I used to struggle badly with, and am still shedding some of those bad behaviors. I would agree 100% that letting TDD drive the needs helps keep the code lean and just what you need.
Jan 25, 2010
This seems similar to a blog post I read a while back (I think thoughtbot may of authored but cannot find a reference). But the idea is to remove relationships (has_many, belongs_to, etc.) that you are not actually using.
BTW, you could probably remove the entire method and just use what is built into rails. Assuming sites are related to users through the model Participant you should be able to do:
site.participants.build(:user => user).save!
Even less code! :)
Jan 25, 2010
Great advice.
Jan 25, 2010
@Eric Anderson: Not using ActiveRecord. Using MongoMapper. The particular relationship I am using is a bit different. Accounts and Sites both have users and all of a sites users are made up of the site’s account users and the users added just to the site.
Jan 28, 2010
This is essentially unrelated, but the first thing I thought when I saw the first code example was “that needs a macro.”
I’m a Ruby guy who’s learning Clojure, and I always knew how much I missed macros. Learning Clojure is confirming that fact.
That said, this example could easily be handled with a macro-like method that takes a block. It’s a shame that standard method creation in Ruby doesn’t use blocks.
class Object
def method_that_takes_ar_id(method_name, &b)
define_method(method_name) do |id_or_obj|
obj_id = id_or_obj.kind_of?(ActiveRecord::Base) ? id_or_obj.id : id_or_obj
instance_exec(obj_id,&b)
end
end
end
class Site
method_that_takes_ar_id(:add_user) do |user_id|
#do stuff
end
end
Thoughts? Do Tell...