Project

General

Profile

Actions

Feature #12697

closed

Why shouldn't Module meta programming methods be public?

Added by bughit (bug hit) over 7 years ago. Updated over 6 years ago.

Status:
Closed
Target version:
-
[ruby-core:77026]

Description

Methods like alias_method, attr_accessor, define_method, and similar

I don't think Ruby discourages this kind of meta programming, so why make it less convenient, by necessitating send or module_eval?


Related issues 2 (0 open2 closed)

Related to Ruby master - Feature #8846: Publicize Module#includeClosed08/31/2013Actions
Related to Ruby master - Feature #6539: public and private for core methodsClosednobu (Nobuyoshi Nakada)Actions

Updated by shevegen (Robert A. Heiler) over 7 years ago

This is somewhat an interesting comment made here, epecially in regards to .send().

Take code snippets such as here:

https://gist.github.com/melborne/665406/4a2a732b1c35b290ddb5a08661453704b8d3c047#file-irc-rb

Quote:

require "term/ansicolor"
String.send(:include, Term::ANSIColor)

I think I myself has used similar ways for .send(), but in another context. I only vaguely remember
that if I would not have used .send() I would have gotten an error about using a private method.

So that error confused me, since I could overrule it anyway by simply using .send() instead - so
essentially, ruby forced me to use a more verbose way rather than the shorter. It was not a huge
issue for me at all since I really like .send() anyway. I also remember the addition of
.public_send() to respect visibility but I myself just happily use .send() since it is shorter. :)

So on this particular problem, I somewhat concur with "bug hit" on principle. But perhaps there
are other reasons why this is not wanted. After all, .public_send() did not exist in the old
days - I do not precisely know how or why or when it was added but I assume that some people
used a strict separation between private/public in their ruby code.

I think that matz may have once said that private/public do not make as much sense in ruby simply
because ruby is so extremely dynamic and flexible. But perhaps there were other reasons too, e. g.
the .public_send() - it is a bit strange though because to me the public/private distinction does
not really add a lot of "necessary things" other than restricting what can be done - but .send()
can bypass this anyway, at runtime, so I am a bit confused. It's sort of what you get when you
have a very flexible language, which is a good thing whenever you want to be flexible.

I mention .send in particular because it was the only one where I noticed the above - I myself
do not use alias_method(), I prefer the shorter alias, even if it is not fully equivalent (I
am sorry, I think being terse is prettier when it is still readable.)

Last but not least, a lot of the meta method stuff seem to lead to fairly long and complex
code, compared to other ruby code which tends to be much simpler - I only mention it since
I have noticed this in my own code too, so I tend to go "oldschool" rather than "super-meta-
clever".

Updated by marcandre (Marc-Andre Lafortune) over 7 years ago

  • Assignee set to matz (Yukihiro Matsumoto)

I made the same proposal years ago (#6539), and Matz stated that he "thinks class/module operations should be done in the scope".

I still strongly believe that include should be public, so I'll keep this open in case we manage to rally Matz

Updated by matz (Yukihiro Matsumoto) over 7 years ago

  • Status changed from Open to Feedback

I still believe

class String
  include Term::ANSIColor
end

is far better than String.include Term::ANSIColor. It is clearer and has more space to optimize.
Besides that the fact that include etc may have huge performance penalty is also a reason to prohibit casual class/module modification. Is there any reason to allow this in addition to saving extra few keystrokes?

Matz.

Updated by bughit (bug hit) over 7 years ago

Yukihiro Matsumoto wrote:

I still believe

class String
  include Term::ANSIColor
end

is far better than String.include Term::ANSIColor. It is clearer and has more space to optimize.
Besides that the fact that include etc may have huge performance penalty is also a reason to prohibit casual class/module modification. Is there any reason to allow this in addition to saving extra few keystrokes?

Matz.

In many meta programming scenarios you are not including or defining literals at top level but expressions and in methods, so it's not


  class Class1
    include Module1
    define_method(:method1)
    attr_accessor(:attr1)
  end

but


def self.do_some_meta_programming(mod, method, attr)
  Class1.send(:include, mod)
  Class1.send(:define_method, method){}
  Class1.send(:attr_accessor, attr)
end

So yes, :send, and :class_eval is less convenient and unnecessary. This is not the way to communicate that meta programming may have a performance penalty, the way to do that is documentation.

Updated by marcandre (Marc-Andre Lafortune) over 7 years ago

Hi,

As stated by bughit, a typical case where we have to resort to send for these is in meta programming, say:

def self.included(base)
  base.send(:define_method, :foo) { ... } unless base.column_names.include?(:foo)
end

Yukihiro Matsumoto wrote:

I still believe

class String
  include Term::ANSIColor
end

is far better than String.include Term::ANSIColor. It is clearer and has more space to optimize.

I respect your opinion. Please realize that not everybody agree with this though. I feel that String.include Term::ANSIColor is clearer as it does one and one thing only. It's a single atomic operation. The class ... form, for me, means "Reopen the class", followed by "include ANSIColor", followed by "ok, that's actually all we wanted to do for this class".

The same way some people will prefer

if some_condition
  do_this_single_thing
end

I usually prefer

do_this_single_thing if some_condition

Typical scenario of a gem: define modules in lib/my_gem/..., then in lib/my_gem.rb:

require_relative 'lib/...'

String.include MyGem::StringExtension
SomethingElse.extend MyGem::Something

That is my preference over

require_relative 'lib/...'

class String
  include MyGem::StringExtension
end

class SomethingElse
  extend MyGem::Something
end

Here are some actual examples of other people that feel this way, from a quick github search:

https://github.com/edgecase/authorize_me/blob/4a1ccf42b2d89a53cafbc0cb4ace236b57ba12ea/rails/init.rb
https://github.com/redradiant/centsible/blob/e4cc35ee1ccb8b1e66992bc18a9b7a26643affe4/vendor/bundle/ruby/1.9.1/gems/recaptcha-0.2.3/lib/recaptcha/merb.rb
https://github.com/nbino/eegloo/blob/a51e6ba77616bc4dd84c76bd0bf07f7ada3a7bd7/vendor/plugins/paginating_find/init.rb
https://github.com/mailserv/mailserv/blob/36b9ff211ba71df0b77aa9de2f6382f60787151b/admin/vendor/plugins/spawn/init.rb
etc.

The search returns over 2.6 million hits (but that includes a lot of duplicates): https://github.com/search?utf8=%E2%9C%93&q=send+include+language%3Aruby+&type=Code&ref=searchresults

Besides that the fact that include etc may have huge performance penalty is also a reason to prohibit casual class/module modification.

I'm afraid I don't see this; I feel that if a programmer is calling include, then it is because include is needed. It will be called no matter what. I doubt this prevented a single misuse of include!

Is there any reason to allow this in addition to saving extra few keystrokes?

As I stated in my original request, I feel that calling send is what should be discouraged, not include. Using public_send is fine, but needing to use send means:

  • I'm doing something I shouldn't be doing
  • I'm calling a method that was not intended for me to call
  • this might break or have unintended consequences, now or in the future

But classes are intended to be augmented, to have methods defined, to have plugins included in them. We shouldn't have to use send to do that.

Actions #6

Updated by mrkn (Kenta Murata) over 7 years ago

Actions #7

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

  • Related to Feature #6539: public and private for core methods added

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

We looked at this issue at todays developer meeting. We referred issue #6539 and now we remember that each method (not the "Module meta programming" at once) should have separate considerations.

While Matz do not like String.include Term::ANSIColor, other methods still have chance.

Updated by marcandre (Marc-Andre Lafortune) over 6 years ago

  • Status changed from Feedback to Closed

shyouhei (Shyouhei Urabe) wrote:

[...]We referred issue #6539 and now we remember that each method (not the "Module meta programming" at once) should have separate considerations.

Module#include is now public (yay :-) ) so I'm closing this.

I'll create separate issues for the others.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0