CODEWITHSUNDEEP
rubyoop
rabidpraxis
rabidpraxis

Reputation: 556

Object Oriented way to deal with complicated method

I have a project that I am working on in ruby and it requires hefty work from some workers. Most of the business logic is contained within these workers and they have become quite complicated. The technique that I have devised models the compose method pattern, but it relies heavily on instance variables to keep state. The way I have set this up seems logical, but I want to get some feedback from the community about if this is wrong, or not the cleanest way I can set this up.

I am essentially using the worker perform (Sidekiq) method as kind of a switch

def perform(transaction_id, data, account_id)
  @transaction_id = transaction_id
  @data           = data
  @account        = Account.new(account_id)

  @campaign_tag   = nil
  @match_str      = nil
  @options        = nil

  has_starter_tag     || return
  find_campaign       || return
  determine_options   || return
  find_active_option  || return
  reservation_over    || return
  already_filled      || return

  send_to_inventory
end

Each method follows the same logic. Check rule, if not satisfactory, perform action (send email, whatever..) and return false, thus halting the execution. If satisfactory, store some ivar data required to complete transaction and return true, thus moving onto the next step.

def has_starter_tag
  result = true

  @campaign_tag = find_starter_tag(@account, @data[:variable])
  if !@campaign_tag
    result = false
    send_email_about_badness
    log_some_stuff
  end

  result
end

To test this code, I stub the instance variables that each method relies on. I understand that this is a code smell since my tests are aware of the implementation instead of the interface. That said, I like the clean interface of these methods, I feel I can scan the source and know exactly what is going on.

If I am doing it wrong, could someone please take some time and explain the right way (or at least another way) to do this? I have always had issues figuring out how to deal with objects that have to perform many small steps that all seem to build upon each other.

Upvotes: 2

Views: 228

Answers (1)

Michael Slade
Michael Slade

Reputation: 13877

My gut feeling is that you are attempting to over-architect this somewhat.

What you describe doesn't count as a design pattern - it's simply separating the functionality required into a number of separate methods. The problem is that the methods are so tightly coupled to each other that separating them doesn't serve much purpose other than to "group" functionality. The only thing the functions seem to have in common is that they check something and cancel the job of the check fails. Repeated functionality aside, You could achieve much the same effect by putting all of the code in one function and separating the individual parts with blank lines.

Somtimes finding some OOP solution or design pattern to use in your code can be counter-productive. At the end of the day what really matters is, does it provide an advantage? Does it save code? Does it make the code easily maintainable and extendable? (Does the code need to be easily extendible?) Don't get too carried away trying to use OOP for the sake of OOP.

Upvotes: 1

Related Questions