2
\$\begingroup\$

I like to reuse code as much as possible and keeps things DRY. But I'm afraid that sometimes it creates too many layers of indirection. It then becomes a chore to find where code exists that is relevant to the problem at hand. I'll give a couple of examples.

In the first example, I'm using inheritance on classes that are not directly related, but one class happens to need a lot of functions from the other class.

Here's a portion of the base class:

class GetTimecard < Interactor
  def date_range_start
    if @request.params[:date_range_start]
      Date.parse(@request.params[:date_range_start])
    else
      pay_period.last_starts_on
    end
  end

  def date_range_end
    if @request.params[:date_range_end]
      Date.parse(@request.params[:date_range_end]) + 1.day
    else
      pay_period.last_ends_on
    end
  end
end

The inherited class is largely the same as base class but does not need a date range. It is only interested in a single day.

class GetTimecardForShift < GetTimecard
  def call
    @request.params[:date_range_start] = @request.params[:date]
    @request.params[:date_range_end] = @request.params[:date]
  end
end

So in the call method you have a strange assignment of variables that only makes sense if you have a look at the base class.

In the second example, I use modules to share common functionality between many classes.

There is a module like this:

module Timeable
  def timesheet_entries_for(shift)
    TimesheetEntryRepository.where(account_and_user_ids.merge(assigned_shift_id: shift.id))
  end
end

This function calls a method, account_and_user_ids which is not defined in the module itself. Instead, it's defined in each class that includes the module. But each class may define it differently. For example:

class RecordTime < Interactor
  include Timeable

  def account_and_user_ids
    { account_id: @request.account_id, user_id: @request.current_user.id }
  end
end

class GetTimecard < Interactor
  include Timeable

  def account_and_user_ids
    @account_and_user_ids ||= { account_id: @request.account_id, user_id: get_user_ids }
  end

  def get_user_ids
    if @request.params[:id]
      @request.params[:id]
    else
      if @request.params[:departments]
        UserRepository.by_account(@request.account_id).where(department_id: @request.params[:departments]).map(&:id) & managed_users.map(&:id)
      else
        managed_users.map(&:id)
      end
    end
  end
end

So it quickly becomes confusing trying to figure out which method is defined where and how changing one method will affect other code.

Are there standard ways of making these dependencies explicit? Hopefully there's a naming convention or a better way to names things. I'm not a fan of too many comments in code because they have a tendency to tell less than the code itself and even to be flat out wrong.

\$\endgroup\$
1
\$\begingroup\$

These dependencies can't be fixed by naming. They are a symptom of some fundamental design problems. If you're having trouble finding methods, that's a symptom of an overly coupled design.

You have a class names that really sound like methods in a completely different class.

Without knowing more about the application it's hard to suggest where to go. If you're working in Ruby I highly recommend this book for thinking about class design and re-factoring.

http://www.poodr.com/

You might also look into various code metric tools to help you identify some design problems. There are lot's of these available for Ruby.

https://www.ruby-toolbox.com/categories/code_metrics

These can seem quite pointless and arbitrary at first, but using SOLID object design principals has been shown to be effective in the long run.

http://en.wikipedia.org/wiki/SOLID_(object-oriented_design)

\$\endgroup\$
  • \$\begingroup\$ These classes represent Use Cases from The Clean Architecture style of system design. The coupling is an unintentional side effect of an effort to make code reusable. I'm looking for pointers on how to reduce the coupling and be DRY at the same time. \$\endgroup\$ – Reed G. Law Jan 28 '14 at 18:44
  • \$\begingroup\$ I'd suggest looking at this article. IMHO, you are being far too literal in mapping Use Cases to objects. ksc.com/articles/usecases.htm \$\endgroup\$ – Fred the Magic Wonder Dog Jan 28 '14 at 20:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.