Look inside

Posted by pratik
on Sunday, July 01

“Don’t use the plugin you cannot write yourself. “

They are to save your time. Oh well, of course everyone is allowed an exception or two.

And I hate it when plugins do something like :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

module Something::ActiveRecord
  module Base
    def self.included(base)
      base.class_eval do
        base.extend(ClassMethods)
      end
    end

    module ClassMethods
      def references_table_name(column_name, options)
        stuff
      end
    end
  end
end
ActiveRecord::Base.send(:include, Something::ActiveRecord::Base)

Instead of :

1
2
3
4
5
6

module Something::ActiveRecord::Base
  def references_table_name(column_name, options)
    stuff
  end
end
ActiveRecord::Base.send(:extend, Something::ActiveRecord::Base)

Makes me not wanna use it.

Comments

Leave a response

  1. Dave – July 01, 2007 @ 06:34 AM

    For those of us less knowledgable, can you explain why one is better than the other?

  2. Alex – July 01, 2007 @ 12:37 PM

    I’d actually argue the other way. 90% of all plugins will not consist purely of class methods and I think its actually more damaging if that 10% follow a completely different pattrn.

    If you decide to add a class method in the second code example, you’re forced to pretty much rewrite the module. In the first code example, the author can just put ”# any instance methods go here”. It may not be as brief but I feel it adds consistency and flexibility.

    But hey, each to his own.

  3. Alex – July 01, 2007 @ 12:39 PM

    Sorry, I meant add an instance method.

  4. Pratik – July 01, 2007 @ 12:46 PM

    Alex,

    Most of the times plugins have code that has to be executed in class scope. Something like alias_method_chain

    Plus, the usual coding practice is to have two modules inside a plugin : InstanceMethods and ClassMethods.

    Also, in my example, you can just do the following.

    
module RedHillConsulting::ForeignKeyMigrations::ActiveRecord::Base
  def extended(base)
    base.include InstantMethods
  end

  module InstantMethods
  end
end

    Which would be more natural way of doing it.

  5. Tore Darell – July 01, 2007 @ 05:58 PM

    A lot of times this is the result of someone mindlessly copying from one of the popular plugins, because they don’t know what’s going on, just that “it works”. But the other plugins had it in there for a reason; for example when you write an “acts_as_something” plugin, which seems to be pretty popular, you don’t want to litter every class with methods it may or may not need, and separating them into ClassMethods, SingletonMethods and InstanceMethods seems to me like a good convention. The “copyer” is often oblivious of all this, of course.

  6. Christoph – July 21, 2007 @ 10:55 AM

    If you make such statements without describing the problem, the post is totally useless. If someone asks for an explanation, why don’t you write it?

    I mean, you complain about n00bs. This is open source, HOW should someone learn a better approach with an article like this? They come across the site, read it and “mkay, but what exactly is wrong with it?” – No answers.

    Very Nice.