Karol Galanciak - Ruby on Rails and Ember.js consultant

Refactoring Tips: Trade Conditionals for Type Delegation

Having some kind of type attribute in your models is a pretty common thing, especially in applications with more complex domain. How do you handle such cases? Is it with multiple conditionals / case statements in every method dealing with the type attribute? If you’ve been struggling with organizing and maintaing code for such use cases then say hello to your new friend: type delegation.

Type attribute - common approach

Imagine you have some Subscription model with type and charged_at columns. Let’s assume that the possible values for type column are daily, weekly, monthly and yearly. Now let’s add couple of methods: next charge date, subscription price and maybe eligible discount which is going to depend on the subscription type as well. To make it a bit more complex let’s assume that for yearly subscription the user needs to select some kind of bonus. The common approach to this problem would be the following:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
class Subscription < ActiveRecord::Base
  validate :bonus_id, presence: true, if: -> subscription { subscription.eligible_for_bonus? }

  def next_charge_at
    charged_at.advance(days: subscription_days)
  end

  def price
    base_daily_price * subscription_days - discount
  end

  def discount
    case type
    when "daily"
      0
    when "weekly"
      100
    when "monthly"
      500
    when "yearly"
      10000
    end
  end

  private

  def base_daily_price
    100
  end

  def subscription_days
    case type
    when "daily"
      1
    when "weekly"
      7
    when "monthly"
      30
    when "yearly"
      365
    end
  end

  def eligible_for_bonus?
    return true if type == "yearly"
    false
  end
end

This class is quite short, but already suffers from the conditionals in many places and as the logic grows, the case and if statements are going to be in even more places, which will be a maintenance nightmare. Are there any ways we could make it better?

The simplest one would be probably using a hash instead of case or if statements. Here’s one example for discount:

1
2
3
4
5
6
7
8
9
10
11
12
class Subscription < ActiveRecord::Base
  TYPES_DISCOUNTS_MAPPING = {
    daily: 0,
    weekly: 100,
    monthly: 500,
    yearly: 10000
  }.freeze

  def discount
    TYPES_DISCOUNTS_MAPPING.fetch(type.to_sym)
  end
end

It looks better than conditionals, but the idea is still the same. It may also grow too complex when you don’t return a simple value based on the type, but perform some computations which would mean having lambdas as a return value per type.

For a real change we could introduce more object-oriented techniques. How about polymorphism and delegation? Every type of subscription looks like a separate concept, maybe we could encapsulate logic related to each type in the separate classes implementing the same interface? That way we would reduce the number of conditionals in all possible cases to just one - in the method where we actually return the type class for given subscription type. Let’s see how it works in practice:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
class Subscription < ActiveRecord::Base
  TYPES_CLASSES_MAPPING = {
    daily: DailySubscription,
    weekly: WeeklySubscription,
    monthly: MonthlySubscription,
    yearly: YearlySubscription
  }.freeze

  validate :bonus_id, presence: true, if: -> subscription { subscription.eligible_for_bonus? }

  delegate :discount, :subscription_days, :eligible_for_bonus?, to: :type_class

  def next_charge_at
    charged_at.advance(days: subscription_days)
  end

  def price
    base_daily_price * subscription_days - discount
  end

  private

  def type_class
    TYPES_CLASSES_MAPPING.fetch(type.to_sym).new
  end

  def base_daily_price
    100
  end

  class DailySubscription
    def discount
      0
    end

    def subscription_days
      1
    end

    def eligible_for_bonus?
      false
    end
  end

  class WeeklySubscription
    def discount
      100
    end

    def subscription_days
      7
    end

    def eligible_for_bonus?
      false
    end
  end

  class MonthlySubscription
    def discount
      500
    end

    def subscription_days
      30
    end

    def eligible_for_bonus?
      false
    end
  end

  class YearlySubscription
    def discount
      10000
    end

    def subscription_days
      365
    end

    def eligible_for_bonus?
      true
    end
  end
end

Now each type is a separate domain concept and it’s really simple to extend any logic based on the type: we would just need to add a method to every class without worrying about ugly conditionals and simply delegate the method call to type_object using delegate macro from ActiveSupport.

You may be wondering whether it actually improved design as there’s more code and classes. The answer is still yes: the object-orientation is all about interaction between many objects and identifying domain concepts almost always result in larger, but more explicit and easier to maintain codebase.

Wrapping up

Type delegation is a simple technique aimed for eliminating conditional logic in different methods that depend on some type attribute and making code much easier to maintain and extend.

Comments