Code Quality Catch-22 -- Duplication vs Complexity

I've started using Code Climate to assess some of my code quality and ran into a situation that I can't seem to find a good solution for. In short, in trying to solve some code duplication that resulted in a low "grade" by using metaprogramming, I simply get a low grade for code complexity instead.

Anyone know a good a way out of this trap?

Code Quality Background Info

Here are some resources that point to the underlying libraries used by Code Climate to assess code quality. I think these are pretty standard in the ruby world so this isn't a Code Climate specific question.

Deciphering Ruby Code Metrics

Tools used by Code Climate

Before Code (duplication problem)

Several associations on User that have basically identical functionality.

has_many :user_conditions
  has_many :conditions, :through => :user_conditions do
    def  :user_symptoms do
    def  :user_treatments do
    def <<(new_item) # disable duplicate addition
      super( Array(new_item) - proxy_association.owner.treatments )
    end
  end
  def activate_treatment(treatment)
    ActiveRecord::Base.transaction do
      self.treatments << treatment
      self.update_attribute(:active_treatments, (self.active_treatments | [treatment.id.to_s]))
    end
  end
  def deactivate_treatment(treatment)
    self.update_attribute(:active_treatments, (self.active_treatments - [treatment.id.to_s]))
  end

After Code (complexity problem)

Moved out of User into a module, admittedly this code is hard to grasp.

```
module UserTrackables
extend ActiveSupport::Concern

included do |base|

%w( treatments conditions symptoms ).each do |trackable|

base.class_eval do
has_many "user_#{trackable}".to_sym
has_many trackable.to_sym, :through => "user_#{trackable}".to_sym do

# disable duplicate addition
def <<(new_item)
super( Array(new_item) - proxy_association.owner.send(proxy_association.reflection.plural_name.to_sym) )
end

end

Your first version - extending the association - is probably the right approach in general, but there's another way to remove duplication, without resorting to metaprogramming.

Instead of defining methods as inlined extensions you can create a module for the association extensions. But rather than adding that module to the User model, you can add it the associations instead (see the docs)

E.g. if you create a module like so:

module TrackableAssociation
  def <<(item)
    # ...
  end

  def activate(item)
    # ...
  end

  def deactivate(item)
    # ...
  end
end

You should be able to add it like so:

class User  { extending TrackableAssociation }
  has_many :symptoms, through: :user_symptoms, -> { extending TrackableAssociation }
  has_many :treatments, through: :user_treatments, -> { extending TrackableAssociation }
end

Which, in turn, should let you do things like

user.conditions.activate(condition)

Now your extensions are defined without using duplication or metaprogramming.

However. Your approach of storing an array of "activated IDs" smells fishy. It's fragile, and it requires the association extensions to modify the user instance. The code paths get tricky.

active should be an attribute on a join model - or at least a column in the join table. If I'm reading your code right, you've got Condition, Symptom and Treatment models, and join tables between them and User. You should probably make actual models for the join; said models can then have an active boolean attribute.

StackExchange Code Review Q#77895, answer score: 4