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.

• m_x: that's more or less what i did with the decorator class. I guess it's a valid approach, but it quickly became really complex as my class has multiple subclasses with different calculations rules - i ended up having one decorator class for each of my classes, plus null decorator classes. That felt a bit too much... And the fact is that i want the thing to be dynamic, i.e. when i call a method it should retrurn the actual calculation result from up-to-date state values - i would have to create a decorator every time i call a method, calling valid? ...
• m_x: I'm currently exploring the Maybe monad option, seems easier and cleaner than expected, i'll let you know. thanks for the input anyway
• Rene Saarsoo: I assumed you can first validate the values and then do the calculations. But when you need to modify the values in the middle of these calculations, my approach starts to fall apart.

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.