These are both great refactorings, but be cautious of classes with an excessive amount of private class or instance methods. This is a smell that often indicates a class is doing too much work. Help maintain cohesion by refactoring private methods into separate objects. In this post, we’ll look at a way of writing private methods that encourages future extraction.

Iceberg Classes and Hidden Abstractions

An iceberg class is a class that has more private methods (3? 5? 10?) than public methods. Private methods are often an indication of additional, hidden responsibilities. By adopting a functional style, private methods can be easily extracted into separate objects.

A private function is a private method that:

Calculates its result only from its arguments

Does not rely on any instance (or global) state

An Example – From Methods, to Functions, to Extraction

Below is a modified portion of the User model from a Ruby on Rails Tutorial sample app.

When a User is created their salt and encrypted password are set. Four private methods are used to implement this.User#encrypt_password is called by ActiveRecord, which means we have no control over sending arguments to it, so it will have to remain a method. Of the remaining three private methods, only one User#secure_hash, is a function.

We're still left with one private class function. This seems ok because it's related to Encryptor's core responsibility. Also, Encryptor.make_salt is not a function because it relies on global state, Time.now; this will make unit testing it difficult. Let's punt on fixing that for now, because this class is already an improvement.

Finally let's update User by having it collaborate with our new Encryptor class.

The Single Responsibility Principle and Object-Oriented Ceremony

There are two disadvantages of extracting private functions into new classes:

Naming the new abstraction is difficult because it's often a verb and not a noun

The message sender now has to now instantiate a class and send it a message

Our above User refactoring resulted in a somewhat awkward, doer class (Encryptor). I also only used class methods in Encryptor, essentially creating a namespace of functions. This eliminates the need to instantiate a separate class, but it doesn't feel very object-oriented.

I don't see a solution for either of these two disadvanages. They're by-products of modeling software in an object-oriented way.

Keeping Classes Cohesive

Cohesive, single responsibility classes are easy to understand and reuse. Private methods are one indication that a class is beginning to take on additional responsibilities. By writing private methods in a functional style, you take the first step in preserving a class's true responsibility.

5 Comments

Lior Sion says:

Hi,

Why did you decide to leave the knowledge of our encryption is done in the User class? It still calls the Encryptor twice, because it knows a salt needs to be created.. won’t Encryptor.encrypt_password(password) make more sense?

July 1, 2013 at 2:22 am

Jared Carroll says:

@Lior,

That sounds like a good refactor. You could make Encryptor.encrypt_password return the salt and password. User would then know even less about Encryptor.

You can add a `attr_accessor :encryptor` to the `user.rb`, and default it to `Encryptor.new`, you can set the encryptor on the go and push the decision later, with this refactor, you can decouple the ‘User` with the concrete `Encryptor`

July 2, 2013 at 10:09 am

Jared Carroll says:

@Hao,

That sounds like a refactor that would add a lot of flexibility. User would just need someone that understands #make_salt and #encrypt. If I inject this dependency, then I could swap out the specific implementation by just changing the default value. I like it!