Would you merge repeating code even when it's get more complex?

ruby-talk
1

Hi, I wrote a module including some instance and class methods with are
very similar. That means some lines do exactly the same once for
instance and once for class methods. I started to refactor theses
methods but the code got quite complex as a result.

Please take a look at this module:
https://github.com/mjio/uberhook/blob/master/lib/uberhook/base.rb#L43

Would you merge these methods to remove the repeating code even when the
code gets more complex and harder to read?

···

--
Posted via http://www.ruby-forum.com/.

2

Uncle Bob commands: extract methods till you drop. Extract your repeated code until there isn't any.

Now, I try not to be religious about this, but I can't think of any examples where having repeated code has been anything other than a maintenance problem. If this is personal code, do whatever. If it's for a company , Bob Martin's advice is well worth considering.

···

On Aug 23, 2012, at 9:34 PM, "Martin J." <lists@ruby-forum.com> wrote:

Hi, I wrote a module including some instance and class methods with are
very similar. That means some lines do exactly the same once for
instance and once for class methods. I started to refactor theses
methods but the code got quite complex as a result.

Please take a look at this module:
https://github.com/mjio/uberhook/blob/master/lib/uberhook/base.rb#L43

Would you merge these methods to remove the repeating code even when the
code gets more complex and harder to read?

--
Posted via http://www.ruby-forum.com/\.

3

Martin J. wrote in post #1073029:

Hi, I wrote a module including some instance and class methods with are
very similar. That means some lines do exactly the same once for
instance and once for class methods. I started to refactor theses
methods but the code got quite complex as a result.

Please take a look at this module:

https://github.com/mjio/uberhook/blob/master/lib/uberhook/base.rb#L43

Would you merge these methods to remove the repeating code even when the
code gets more complex and harder to read?

I haven't looked to thoroughly but for example this calls for
simplification:

      def run_instance_callback(type, target, scope)
        return unless instance_hooks[target].has_key?(type)
        scope.instance_eval(&instance_hooks[target][type])
      end

      def run_class_callback(type, target, scope)
        return unless class_hooks[target].has_key?(type)
        scope.instance_eval(&class_hooks[target][type])
      end

->

      def self.run_callback(type, target, scope, hooks)
        return unless hooks[target].has_key?(type)
        scope.instance_eval(&hooks[target][type])
      end

->

      def self.run_callback(type, target, scope, hooks)
        tp = hooks[target][type] and scope.instance_eval(&tp)
      end

Looking a bit further it seems you want to put this in a class
hierarchy:
Hooks
ClassHooks < Hooks
InstanceHooks < Hooks

Then apply "template method pattern" e.g. for the different conditions
in method instance_hook and class_hook.

Pushing this further you then only retain method instance_hooks and
class_hooks which return a class of the corresponding type. Instead
calling instance_hook(...) {|..|} someone will invoke
instance_hooks.hook(...) {|...|}. You could as well keep instance_hook
but create forwarding in a simple way, e.g.

%w{hook process_method_added}.each do |name|
  %w{class instance}.each do |scop|
    class_eval "def #{scope}_#{name}(*a,&b)
#{scope}_hooks.#{name}(*a,&b) end"
  end
end

Kind regards

robert

···

--
Posted via http://www.ruby-forum.com/\.

4

I think the problem here is confusion about object system. I don't see any
reason to explicitly handle instance methods and class methods. When you
remove this requirement, the duplication goes away.

I was going to try refactoring it to show you what I mean, but the tests
can't accommodate that, they test implementation instead of behaviour (each
method is explicitly tested, so if I don't have those methods doing those
exact things, then I'll fail the tests). So instead, I wrote an example
from scratch: Creates before/after hooks for methods · GitHub

···

On Thu, Aug 23, 2012 at 10:34 PM, Martin J. <lists@ruby-forum.com> wrote:

Hi, I wrote a module including some instance and class methods with are
very similar. That means some lines do exactly the same once for
instance and once for class methods. I started to refactor theses
methods but the code got quite complex as a result.

Please take a look at this module:
https://github.com/mjio/uberhook/blob/master/lib/uberhook/base.rb#L43

Would you merge these methods to remove the repeating code even when the
code gets more complex and harder to read?

--
Posted via http://www.ruby-forum.com/\.