Code reuse while keeping meaning clear and avoiding unforseen consequences

Question

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.

Answer 1

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)

• Reed G. Law: 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.
• Fred the Magic Wonder Dog: 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