Ruby OO design : how to handle inconsistent state in a mutable class?

Question

Abstract

I'd like to have advice on how to :

• implement methods that rely on a state,
• when the instance is mutable and may be in an inconsistent state,
• in such a way that the methods either silently fail or return nil,
• without having to nil guard / coerce state to meaningful values everywhere.

Anyway, if my design is wrong itself, let me know.

Context

I have an ActiveRecord class. It is pretty simple, but has lots and lots of methods that require the whole instance to be valid? to work correctly.

The class in question handles a complex tax calculation, but for the sake of clarity, let's boil it down to this :

class Foo < ActiveRecord::Base

  validates :a, :b, :c, :d, :e,
            numericality: {inclusion: 0..100}
  validates :f, :g, :h,
            numericality: {greater_than: 0}

  def some_method
    a * b / c
  end

  def other_method
    d + e * (g / f)
  end

  def yet_another_method
    some_method - other_method * f
  end

  def this_is_getting_really_complex
    yet_another_method * other_method - some_method
  end

  # and so on, with piles of methods calling each other
end

Moreover, i have subclasses for this class that override these methods (different tax calculation rules, etc.)

The thing is, as long as an instance is in a consistent state (all a, b, c... fields are present and valid), all the methods work fine. But ActiveRecord instances must be able to be in an inconsistent state, because...well, we want to be able to validate them. So raising ArgumentError during initialize when the params are meaningless is out of the question.

In this case, let's say a, b, c are filled with junk strings or nil instead of meaningful values : all of our methods will raise various errors, or even worse - thanks to duck typing it will work in a crazy, unintended way ( "junk" * 300_000, anyone? ).

Trying to solve this problem

first try : nil guards

this is getting ugly pretty quick :

def some_method
  return nil unless [a, b, c].all? &:present?
  a * b / c
end

... ugh, nil guarding isn't enough, what if i have strings instead of numbers ? I admit for a while i was tempted with heresy :

def some_method
  a * b / c
rescue TypeError, NoMethodError
  nil
end

This is too bad. Wait, what if i simply check for validity ?

def some_method
  return nil unless valid?
  a * b / c
end 

A bit better, but now the whole validation process kicks in every time i call a method. This is silly. OO to the rescue ?

second try : OO refactoring

My first thought was : well, we have a behavior that varies according to state - this is textbook example for a state machine. But how would i hook this on AR's validation cycle ? Did not figured it out.

Then I proceeded to try and extract those methods into a variety of immutable decorator classes, with a factory method that accepted one instance from my Foo class :

 class FooDecorator

    def self.factory( foo )
      # NullDecorator has methods that always return nil
      return NullDecorator.new( foo ) if foo.invalid? 
      case foo
        when Foo:Bar then BarDecorator.new( foo )
        when Foo:Baz then BazDecorator.new( foo )
        else raise ArgumentError
      end
    end

    def initialize( foo )
      @foo = foo.dup.freeze.readonly!
    end

    def some_method
      @foo.a * @foo.b / @foo.c
    end

    def yet_another_method
      some_method - other_method * @foo.f
    end

  end

I promptly stopped because it felt ridiculously overengineered, and smelled like feature envy over the top.

I also considered using the Maybe monad, or create a monad of my own like "MaybeAValidNumber" to coerce everything to meaningful values. That felt not much better than nil guards and again, overengineered.

third try : don't care

Thinking about this, i wondered if i should nil guard at all : after all, these methods are only relevant if the instance is in a consistent state. As far as i understand it, design by contract goes this way : if you do not ensure state prerequistes are met before calling the methods, you break the contract, so you are at fault.

Problem is, as you can easily guess, these methods are likely to be called in the views to display the results of calculations. I don't find having to throw a bunch of conditionals in my views entirely satisfying...

Thoughts ?

Answer 1

Looks like your class has two separate responsibilities:

• validating a set of fields (fields can be in an inconsistent state)
• performing calculations on these fields (fields must be valid)

Therefore I would split it up into two classes:

• The ActiveRecord could still take care of the validation and other ActiveRecord stuff.
• All the tax calculations though would happen in say TaxCalculator, which initializes all its fields from a validated ActiveRecord that's passed in from a constructor, so all the methods of it can blindly assume that the fields are OK.

Answer 2

Accepted an answer because it is a valid solution, but i'd like to explain an approach that seemed interesting (not functional as is): coercion plus a NullObject, in the spirit of the Maybemonad used by Avdi Grimm.

 class NonFloat

   include Comparable

   # allows arithmetic operations to take place with non-null values
   def coerce( other )
     [self, self]
   end

   def to_f
     self
   end

   def coerce( other )
     [self, self]
   end

   # did the same thing for various operators
   def *( other )
     self
   end

   def to_s
     ""
   end

   def <=>( other )
     # yuck, could not find anything meaningful to return from here :/
   end
 end

 def MaybeFloat( value )
   return value if value.kind_of?( NonFloat )
   Float( value )
 rescue TypeError # coercion failed
   NonFloat.new
 end

... Looks oddly like a NaN ;). What i like about this solution is that it allows to perform arithmetic operations, even with potentially non-float coercible values :

 MaybeFloat( value ) * MaybeFloat( other_value )

but more important, this behavior can propagate :

 def some_value
   MaybeFloat( @value )
 end

 def some_calculations
   some_value * 4 + other_value
 end

 def other_calculations
   some_calculations / yet_another_value # may return a NonFloat
 end

what s*cks is that you can't really compare NonFloat with other types because <=> is not expected to return anything other than 1,0 and -1 (no way to tell "it doesn't make sense to compare these values")... so Array#min, for instance, can't be used.