ryanjm / rails-event-manager

Repeating schedule manager Rails.
MIT License
0 stars 0 forks source link

Don't include private methods in module #8

Open ryanjm opened 10 years ago

ryanjm commented 10 years ago

Right now all the methods from the schedule module are included in the model/class that EventManager gets added to. I want to limit it to just the "public" methods.

ryanjm commented 10 years ago

@barnes7td I'd like to get your thoughts on this when you get a chance.

I'd like to split up public vs private methods as far as what the user of the gem experiences.

Option 1. is to put a private section inside the module. My concern with that is that I do want to test those methods and private methods aren't the easiest to test directly.

Option 2. we could move the "helper" methods into their own class and then have the main "public" methods just use that class directly. Something like this:

module MyModule

  module Schedule
    def events_between(a,b)
      ScheduleHelpers.next_event_after(a)
    end
  end

  class ScheduleHelpers
    def self.next_event_after(a)
      a+1
    end
  end

  module Validations
    def is_valid?
      true
    end
  end

  # When MyModule is included
  def self.included(base)
    base.send :include, Schedule
    base.send :include, Validations
  end
end

class Test
  include MyModule
end

I forget what you said about that concept of testing only public methods. It makes sense to me, but since there are so many moving parts right now, I'd like to keep tests for them. In a separate class the tests could still work for that.

The other benefit of moving them into a class would be that we can split the tests up into separate files and have more rigorous testing on those main public methods. Then in the future those helper class tests could (optional) be dropped.


I want to figure out what the best strategy is for this and then apply it to all of the other sections of code that we'll have in the future (in the short term that would be a section for validation).

barnes7td commented 10 years ago

I do agree with the test public methods only (with some exceptions). However, we are in refactor mode and that process can get messy. I think the end goal should be to test only the public methods, but testing for assurance of not breaking anything and testing that if functions correctly on any methods necessary is needed during this phase.

In the end, my general solution for testing only public would be to put things into a new class but I would save judgement to each situation.

I think it is going to be messy for awhile and then it will start coming together. I think it is most important to get the domain object (classes) and naming right and the design will clear itself up.