RubyFlow The Ruby and Rails community linklog

Avoid `self.included(by) by.extend ClassMethods`, use inherit!

by apeiros —

I very much dislike the all too common pattern <pre>module Foo def self.included(by) by.extend ClassMethods end end</pre> That’s why I created the inherit gem. It’s minimal (15 LoC relevant code). Please check it out! Feedback is welcome!

Comments

so, you’ve replaced the “anti-pattern” (and I quote because it isn’t) with another: extending the Object class. Well done.

jemmy —

You omitted the more important one: Module. But you’re correct. I’ve replaced the N occurrences of an anti-pattern with 2. I also replaced implicit, hidden code with explicit meaning. An acceptable trade-off in my opinion. Also, don’t forget that something as prominent as rubygems started as a monkey-patch of Kernel#require (it still is, but given that it is now in Core, I wouldn’t consider it as one).

apeiros —

How is this different then ActiveSupport::Concern?

lhm —

@lhm ActiveSupport::Concern doesn’t do away with the anti-pattern. You still use class Foo; include SomeConcern; end. That the include performs more modifications to the class than just the inclusion of the module is still hidden with it.

apeiros —

@lhm Other differences (without a judgment): AS::Concern manages depending modules, inherit does not. AS::Concern provides an included do … end block, inherit does not. AS::Concern adds ivars (@_dependencies and @_included_block) to a concern, inherit does not. Inherit monkey patches Module and Object, AS::Concern does not. AS::Concern includes the given module (with the consequence of e.g. ThatModule::ClassMethods also ending up in the target class), inherit includes ::InstanceMethods and ::Constants instead.

apeiros —

We’ll have to agree to disagree on this one. I do agree that you’ve wrapped up a common idiom, and there’s nothing wrong with your implementation. Except, you’ve called it “inherit” which has a particular meaning.

But I have an issue with the rising use of the term “anti-pattern” to describe anything a particular programmer disagrees with. It’s not an anti-pattern to hook into included in your own module.

If there is an anti-pattern (and I’m not saying it is) its extending a class when a module is included, and your gem perpetuates that.

jemmy —

@jemmy nice that after your passive-agressive but otherwise substanceless first post you managed to write something of substance. I agree that the use of anti-pattern is used inflationary and not always correctly. self.included/extended has by its widespread usage become a pattern. It is an anti-pattern because it violates two things: a) the callback mutates its argument. You generally only want methods to mutate the receiver, not its arguments. Inherit does that correctly. b) because include has a very specific meaning which is warped and makes code harder to follow. It’s one case which is commonly considered as “magic”. I chose inherit as a name because that’s exactly what it does. An include ends up in the ancestry of a class already (inheritance). It’s “only partial” inheritance in the sense that it only gets you the instance methods. A normal class inheritance gets you class methods too. And that’s what you get with Module#inherit.

apeiros —

@jemmy note that I have no problem with you disagreeing. But your first post was just inflammatory and no base for any reasonable discussion.

apeiros —

My first post has a very good discussion point, you just haven’t discussed it. I can put it another way - I consider extending Object, and as you pointed out, Module, far worse than the problem you’re trying to solve.

Imagine someone wrote a gem depending on yours and I include that gem into my codebase. Now you’re extending all MY stuff.

Then I’m messing about in the console and I take a peek at the methods I have available and see “inherit”. Looks like a ruby feature.

With ActiveSupport::Concern you have to extend your module with it. And if you use the included hook I can look at a library and see right there in the code what it will do to the class I include it in. Hiding that away behind a new, and confusing, word is a Bad Thing.

In your follow up arguments, I don’t think (a) is worth discussing because it’s the ideal vs the pragmatic, but (b) I agree, but your library is not a solution. The solution would be to write in such a way that the user of a library can choose to include and extend as they wish. I might not want your finder methods on my class, just the instance behaviour.

jemmy —

@jemmy I’m not going to argue about the tone of your first post. It’s obvious enough and a bit low that you’re trying to redress it as a genuine argument. But it’s up to you how you want to come across.

I’m not concerned about the module you extended with ActiveSupport::Concern. That part is transparent. I’m concerned about the location where you use include. That part is not transparent and AS::Concern does not change that. All AS::Concern does is normalize/standardize the pattern (which is good). I don’t see a problem in the monkey patching of Object/Module in this case. It is an anti-pattern, it should be done carefully (in my >10y of ruby, that’s the first time I do it on a library level - I am careful), but it can make sense (as said earlier, cf. rubygems). Once ruby 2.1 and refinements catch on, that issue is further alleviated by reworking it into a refinement. I’m just not yet quite sure how to prepare it for this in a backwards compatible way.

I disagree with your reasoning for dismissal of a). And for b), my library doesn’t stop you from cherry picking. It’s even better than AS::Concern in that regard. It I) normalizes and II) doesn’t stop you from manually including/extending what you want (and thanks to I) you even know where what is located). With AS::Concern OTOH you’re still stuck with “if you include it, it’ll automatically extend”.

apeiros —

No, I’m not trying to redress it, my first post was dismissive and snarky. I’m good with that.

AS::Concern will only extend if the ClassMethods module is there, much as your code. When Concern was added to ActiveSupport I remember the introductory blog talking of how having the ClassMethods module isn’t ideal, but it’s a continuation of the convention we’re already used to.

My main argument, that you seem loath to reply to, is that you’ve hidden away the details of what is happening behind a new and loaded keyword (inherit): “I also replaced implicit, hidden code with explicit meaning”.

Say I came across a library and it said that to use it I should “include Snide::Comment” into my class. I usually casually browse the code of libraries I’m interested in, and in that Snide::Comment module I’d see def self.included(mod) with hopefully a nice comment above it telling me what it is hoping to achieve. Or I’d see “extend ActiveSupport::Concern”, and I know what that does.

Now, consider the case of a gem making use of inherit. I look in the module but I can’t figure out how it works. The README tells me to use “inherit Snide::Comment”, which is unfamiliar, sounds like existing oop notation, and at this stage I still don’t know how to find out about it (you can make the same argument for AS::Concern, but at least it’s there in the code so we know something is going on).

Finally, I feel that you’re arguing for an include+extend. But I personally feel that the ClassMethods convention we use (I use it all the time) isn’t good enough.

jemmy —

“you’ve hidden away … what is happening” - errr, yes, that’s why we write methods. If you want to know what a method does, you look up its documentation. By that measure every method in every lib is “a new, and confusing, word”. I find that argument rather pointless. If you’re confused and don’t know what to do when you come across an unknown method, well, then you have indeed an issue, and it’s not with inherit.

I’ll repeat an issue I have with AS::Concern and included/extend solutions is that I have to second guess every include statement. It’s not obvious from the include statement alone, that it will also perform an extend.

I’m also a bit baffled by your meandering argumentation with regards to ClassMethods. Once you have a ClassMethods module in your module, AS::Concern does not let you opt out of it. You can’t include that module without getting ClassMethods too. With inherit, you can still cherry pick.

The other points I’ve already covered in earlier answers.

I think I’ve made my case and responded to all your arguments. I don’t think I’ll continue this discussion.

apeiros —

If you find yourself using this pattern a lot, it makes sense to encapsulate it, but I do feel pretty strongly against monkey patching it into Object like this. It would be nice if it didn’t do so by default, then you could just document that it could be done that way if you wanted it everywhere, or you could limit it to just AR::Base or some such subset of the hierarchy.

The terminology is not made more-clear to me than writing my own included method when needed (which is rarely), but I realize that’s subjective. I generally agree with jemmy’s points, but the tone of that discussion was less constructive than it could have been.

Soulcutter —

Rory mcilroy nike air max 90 so that nike shoes online you can cut christian louboutin men shoes american weddingur's 'flagship'

Idea: By 2014, Morneau been nike outlet very useful nike sneakers open up Lebron 11 the beats by dre cheap actual Morneau Shepell Secondary planned for females into Kenya's Kakuma outlet nike refugee cheesy. The teachers trains Somali nike running shoes and nike factory store thus Sudanese christian louboutin for men earlier nike shoes online childhood days. Treaty amount in cheap red bottom heels addition to First nations around the world prime. The research, nike clearance Your nike outlet mom testified that so nike store usa that you can TheLipTV. Having jordan 13 several these jordans for sale court lawsuits, It delivers the nike online store special nike clearance of the inappropriate dedication lawsuit. They nike shoes online solely specialized in your.

Sites label and clearly state bilities. nike online store Area Lebron James Sneakers intending nike store equipments nike air max very often include these christian louboutin store individuals cheap red bottom shoes points of interest christian louboutin heels caused outside of <a title="nikes on sale" href="ht

loemrntdherid1 —

Post a comment

You can use basic HTML markup (e.g. <a>) or Markdown.

As you are not logged in, you will be
directed via GitHub to signup or sign in