What is method gravity?
What I call Method Gravity means for me:
The bigger the method the more new lines of code will be added to it.
I noticed this while working on various projects: once a method grows, the chances that the next developer will add more lines to it increase.
There is a tipping point (like with the gravity of a start) when a method is too big, it will collapse, meaning someone will take it apart and split it into smaller methods.
What is a long method, and what is a short method?
I am not sure there exists a definition of what is a long method and what is a short method. This is subjective (depending on individual or team preferences).
I could say this (citing Sandi Metz): anything bigger than 5 lines of code could be considered a long method.
But I think a better definition could be along the following lines:
If you need to read twice, a method to remember/understand each line of it is long.
If you need to read it twice to understand what it does is (probably) too long (or could use some renaming)
Thus a short method can be understood quickly at a glance.
The main problems with long methods
There are three main problems:
It makes it easy to break Single Responsibility Principle: a long method tends to do many things, and to summarize it to a main purpose means to keep expanding that goal until it reaches a high complexity
It makes it easy to break Cohesion: a long method will tend to do unrelated things resulting in unwanted coupling
A long method tends to favor hard-to-read algorithms or too creative naming to avoid collision or variable name re-use
Benefits of short methods
Simplicity
Single Responsibility Principle: short methods are easy to focus on one single thing and thus are also easy to describe in a simple way
Limits the number of changes: when you have to change something, you can isolate the change to a small method
Easy to test: a small method is easy to test
But for me, the most significant benefit for the developer is that using smaller methods forces us to write better names.
Let's take an example (please read this example as pseudo-code, I will not focus here on language specifics, and the purpose of the example is to show the high-level code design and not focus on specifics)
If you look at the following code, can you say quickly what the result might be?
# input = [{ "slug" => "one_day", "language" => "en"} => [ { "id" => 10, "status" => "booked" }]]
def transform(input)
events_hash = input.flat_map { _1.keys }
event_slugs = events_hash.collect { _1["slug"] }
events = Event.where(slug: event_slugs)
registrations_hash = input.flat_map {_1.values }.flatten
registration_ids = registrations.collect { _1["id"] }
registrations = Registration.where(id: registration_ids)
hash = input.pluck(*KEYS).to_h
hash.transform_keys! { |key| events.find { |e| e.slice(:event_type, :language) == key } }
hash.transform_values { |value| registrations.find { |r| r.slice(:id, :status) == value} }
end
You can probably guess with a bit an effort. And I bet that letting 2 weeks pass and looking at this again, you might still need a small effort to remember.
What about the following code:
def transform(input)
keys_to_event_objects(input)
values_to_registration_objects(input)
end
I think reading this second version of the function would give you a good idea about what transform
the method is doing.
You might wonder what the full code looks like (some spaces and returns were removed to keep the code terse in this article):
def transform(input)
keys_to_event_objects(input)
values_to_registration_objects(input)
end
def keys_to_event_objects(hash) = hash.transform_keys! { event(_1) }
def values_to_registration_objects(hash) = hash.transform_values { registration(_1) }
def event(key) = events.find { _1.slice(:slug, :language) == key }
def registration(value) = registrations.find { _1.slice(:id, :status) == value }
def events(input) = @events ||= load_events(from_event_slugs(input))
def load_events(slugs) = Event.where(slug: slugs)
def from_event_slugs(input) = input_keys(input).collect { _1["slug"] }
def input_keys(input) = input.flat_map { _1.keys }
def registrations(input) = @registrations ||= load_registrations(from_registration_ids(input))
def load_registrations(ids) = Registration.where(id: ids)
def from_registration_ids(input) = input_values(input).collect { _1[:id] }
def input_values(input) = input.flat_map { _1.values }.flatten
This code has a huge advantage: most of the changes you can think of would be limited to a small function. You can achieve the same result with normal methods containing one single line.
Example of possible changes:
include/eager_load on
Event
/Registration
if more attributes need to be used further down the road, => change theload_*
methodsspeed up the
find_*
by ordering records in some specific way (eg, if in general, the input will have the most recent registrations/events) => change theload_*
methodsadding/removing keys from the input => change
event
orregistration
methodssay the API will decide to return
id
for events instead ofslug
this is a breaking change, but still the changes in code are limited to smaller areas => renamefrom_event_slugs
, change the key inside, and then change inevents
For all these changes, the transform
method does not change. This is good because the main algorithm (get hash, map keys to objects, map registration to objects) remains the same.
With small methods, we achieve what Sandi Metz describes as the purpose of design: reduce the cost of change.
Ideas to keep methods small
Some simple things that could help to keep the method gravity small in Ruby:
Use endless methods
# events_controller.rb
def show = render locals: { event: }
# time_rules.rb
def past?(event) = event.finish_time <= Time.current
Why:
- Because once a method is endless, it takes a more significant effort to add another line (you need to make it a normal method)
Use guard clauses
def ensure_access_is_allowed
return if current_user.admin?
current_user.allowed_to?(:access, event)
end
Why:
- Because adding another branch to a guard clause takes a bit of effort, it means transforming the guard clause into a normal
if
condition
Use conditional at the end of the statement
def show
set_context if context_given?
render locals: { user: }
end
Why:
- Because also, here, adding another branch means a bit more effort to transform the end of the statement
if
into a normalif
More general advice
In general, I like the advice from a paper published by Google, called "Searching for Build Debt: Experiences Managing Technical Debt at Google"
So the main idea, if you want to keep the methods small, is to make it hard for you or a colleague to make them bigger in the future. Add a bit of antigravitational force to them.
The same happens in the example I showed above: using endless methods is a bit hard to add one more line to them. You can still rename them and easily change what each of them is doing.
In case you like this recommendation from Sandi Metz:
Methods can be no longer than five lines of code
Then you know what method has always fever than 5 lines of code?
-> an endless method
Disclaimer
I wrote this as general advice. I agree there are cases when a long method makes sense. Maybe. But that should be an exception.
Apply with care! Moderation is key. Do not abuse!
Even Sandi Metz says (regarding her famous rules):
There are actually six rules, and the sixth rule is that you can break any of the first five, as long as you can get your pair to agree. Why is it that we’re such cargo culters about it? It’s like, that’s the rule that people forget.
Update
1. There are some good comments in this Reddit thread and I am planning to update the examples in this article. The comments and alternative solutions proposed there at about the code example. But the main idea of my article I think remains the same: a long method will attract more code.
If you like this type of content, you may want to consider subscribing to my curated newsletter Short Ruby News where I cover weekly Ruby news from around the internet.
Top comments (0)