Today, I’m pairing, but not the way you might expect. I’m pairing craft beer with craft code. The beer is Ithaca Brewing Company’s Creeker - a juicy, malty, bready double IPA that clocks in at 8.5% ABV and reminds me of a stronger version of Heavy Seas’s Loose Cannon. It smells great!

The code does not smell great. It’s an example of primitive obsession, which is, ironically, not a cologne. It’s a code smell that arises when primitives (e.g. strings, integers, etc.) are used in place of objects.

Let’s say we have a person class of which instances have a name and a phone number.

class Person
  attr_reader :name, :phone_number

  def initialize(name:, phone_number:)
    @name, @phone_number = name, phone_number
  end
end

person = Person.new(name: 'Ford', phone_number: '210-690-4242')
person.name #=> Ford
person.phone_number #=> 210-690-4242

Now, let’s add a new requirement. We want to know if a person has a valid phone number. We can accomplish this by adding a method that checks if the phone number matches a common format.

class Person
  # ... code omitted ...

  def valid_phone_number?
    patterns = [
      /^\d{10}$/, # XXXXXXXXXX
      /^\d{3}-\d{3}-\d{4}$/, # XXX-XXX-XXXX
      /^\d{3}\.\d{3}\.\d{4}$/, # XXX.XXX.XXXX
      /^\(\d{3}\)\s{0,1}\d{3}-\d{4}$/, # (XXX)XXX-XXXX, (XXX) XXX-XXXX
    ]

    patterns.any? { |p| p.match? phone_number.to_s }
  end
end

ford = Person.new(name: 'Ford', phone_number: '210-690-4242')
ford.valid_phone_number? #=> true

arthur = Person.new(name: 'Arthur', phone_number: '555-1212')
arthur.valid_phone_number? #=> false

This works, and the person class is still small. However, the person class arguably should not know how to validate a phone number. The person class should only be responsible for things directly related to a person. What if we want to convert the phone number to different formats? Should the person class hold those methods? By using a primitive (i.e. a string) to store the phone number, we’re left with no good place to put the responsibilities related to working with phone numbers. This is a case of primitive obsession.

To fix it, let’s create a new abstraction - a separate phone number class. We can move the phone number validation code there.

class PhoneNumber
  def initialize(value)
    @value = value
  end

  def valid?
    patterns = [
      /^\d{10}$/, # XXXXXXXXXX
      /^\d{3}-\d{3}-\d{4}$/, # XXX-XXX-XXXX
      /^\d{3}\.\d{3}\.\d{4}$/, # XXX.XXX.XXXX
      /^\(\d{3}\)\s{0,1}\d{3}-\d{4}$/, # (XXX)XXX-XXXX, (XXX) XXX-XXXX
    ]

    patterns.any? { |p| p.match? @value.to_s }
  end
end

phone_number = PhoneNumber.new('123-123-1234')
phone_number.valid? #=> true

We can remove the validation code from the person class and delegate the validity check to the phone number.

class Person
  attr_reader :name, :phone_number

  def initialize(name:, phone_number:)
    @name, @phone_number = name, phone_number
  end

  def valid_phone_number?
    phone_number.valid?
  end
end

Now, when we create people, we can give them phone number objects instead of strings.

ford = Person.new(name: 'Ford', phone_number: PhoneNumber.new('210-690-4242'))
p ford.valid_phone_number? #=> true

arthur = Person.new(name: 'Arthur', phone_number: PhoneNumber.new('foo'))
p arthur.valid_phone_number? #=> false

Being able to identify and alleviate code smells is one of the most important tools in your refactoring toolkit. If you are not familiar with the topic of code smells and/or refactoring, Source Making is a great reference. Cheers!