Close
Glad You're Ready. Let's Get Started!

Let us know how we can contact you.

Thank you!

We'll respond shortly.

LABS
Write Private Functions Not Private Methods

During refactoring, private methods are created in order to:

  • Eliminate duplication within the class
  • Clarify confusing and/or complex fragments of related code (Extract Method)

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.


class User < ActiveRecord::Base
  attr_accessor   :password
  attr_accessible :name, :email, :password, :password_confirmation

  before_create :encrypt_password

  # public methods...

  private

  def encrypt_password
    self.salt = make_salt
    self.encrypted_password = encrypt(password)
  end

  def make_salt
    secure_hash("#{Time.now.utc}--#{password}")
  end

  def encrypt(string)
    secure_hash("#{salt}--#{string}")
  end

  def secure_hash(string)
    Digest::SHA2.hexdigest(string)
  end
end

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.

Let's refactor the other two into functions.


class User < ActiveRecord::Base
  # same implementation as above...

  private

  def encrypt_password
    self.salt = make_salt(password)
    self.encrypted_password = encrypt(salt, password)
  end

  def make_salt(password)
    secure_hash("#{Time.now.utc}--#{password}")
  end

  def encrypt(salt, password)
    secure_hash("#{salt}--#{password}")
  end

  def secure_hash(string)
    Digest::SHA2.hexdigest(string)
  end
end

Encryption doesn't feel like a User responsibility, so let's extract it into a separate object.


class Encryptor
  def self.make_salt(password)
    secure_hash("#{Time.now.utc}--#{password}")
  end

  def self.encrypt(salt, password)
    secure_hash("#{salt}--#{password}")
  end

  def self.secure_hash(string)
    Digest::SHA2.hexdigest(string)
  end

  private_class_method :secure_hash
end

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.


class User < ActiveRecord::Base
  # same implementation as above...

  private

  def encrypt_password
    self.salt = Encryptor.make_salt(password)
    self.encrypted_password = Encryptor.encrypt(salt, password)
  end
end

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.

Comments
  1. 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?

  2. 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.

  3. Hao says:

    actually, you can add a `attar_accessor :encryptor` to the `User`, make it default to `Encryptor.new`, with this refactor, you can decouple the `User` with the concrete encryptor

  4. Hao says:

    @Jared,

    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`

  5. 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!

Post a Comment

Your Information (Name required. Email address will not be displayed with comment.)

* Copy This Password *

* Type Or Paste Password Here *