Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain why def self.foo is preferred over class << self #7

Closed
alexanderadam opened this issue Mar 14, 2017 · 6 comments
Closed

Explain why def self.foo is preferred over class << self #7

alexanderadam opened this issue Mar 14, 2017 · 6 comments

Comments

@alexanderadam
Copy link

alexanderadam commented Mar 14, 2017

As your styleguide recommends

class TestClass
  # bad
  class << self
    def first_method
      # body omitted
    end

    def second_method_etc
      # body omitted
    end
  end

  # good
  class << self
    attr_accessor :per_page
    alias_method :nwo, :find_by_name_with_owner
  end

  def self.first_method
    # body omitted
  end

  def self.second_method_etc
    # body omitted
  end
end

I was wondering why. If it's just a matter of taste for you: fine, please write this in the styleguide as well.

It isn't obvious here why you are preferring this because especially in cases with many class methods the block and therefore also the additional indention give a good visual hint that methods are encapsulated and belong together. They share a similar context and blocks are the usual way of creating contexts.

Besides from that it can also help to avoid to have class methods spread over a file. It is easier (at least for people I know) to have class methods together in one place than having a bunch of instance methods in between.

The Eigenclass context also makes sense in your example as per_page and first_method both will create class methods. But in your example just attr_accessor and alias_method are using the Eigenclass context whereas first_method and second_method_etc give an 'defined from the outside' feeling.

These points makes it more understandable for me to follow bbatsovs / rubocops styleguide which says

  # Also possible and convenient when you
  # have to define many class methods.
  class << self
    def first_method
      # body omitted
    end

    def second_method_etc
      # body omitted
    end
  end
@rmosolgo
Copy link
Contributor

rmosolgo commented Sep 5, 2018

I don't agree with the preference expressed in the guide, but I think I understand the issue. Consider this Ruby code:

class A 
  class << self 
    attr_accessor :some_attribute 
  end 
end 

class B < A 
end 

As far as I know, that's the only way to support usage like this:

A.some_attribute = :value_1 
B.some_attribute # => nil 
B.some_attribute = :value_2 
A.some_attribute # => :value_1 

I mean, using a class variable would cause it to be shared by A and B, which we don't want.

Does that sound right to you? Is there another way to implement that snippet above?

I think the same thing is true for alias, it only works for class methods inside class << self.

So, I don't agree with the preference, but I think that explains the difference, does that make sense?

@alexanderadam
Copy link
Author

alexanderadam commented Sep 5, 2018

@rmosolgo first of all: thank you for responding! 🎉
That's awesome!

However, I just wrote about

  def self.foo
    
  def self.bar
    
  def self.baz
    

vs

  class << self
    def foo
      
    def bar
      
    def baz
      

The case that you alias class methods or use class variables shouldn't occur in every class, right? Especially as class variables are kind of dangerous.

I rather would accept if some class would exceptionally not comply to a style guide than having a less readable variant established as a standard in a style guide.

Even if someone decided that this is the way to write class methods on GitHub, and added it to the style guide because of the points you just mentioned it should probably contain an explanation à la "Hey folks, we write class methods this way because we use class method aliasing and class variables so often".

Besides from that: these aliases don't mention the class methods at all. Are you sure that we are talking about the same issue here?

I really just opened this issue to write about the method definition of the class methods.

@maxim
Copy link

maxim commented Sep 20, 2018

I second this, would actually love to have an optional cop for enforcing the class << self style as opposed to self.foo. Summary of benefits:

  • indent serves as the visual que
  • helps keep all class methods grouped together
  • searching def foo returns both class and instance methods
  • adding class-level code requires class << self anyway, might as well use it for methods too

Edit: didn't realize coming from google that this was github's styles, not rubocop itself. Still might as well throw in my 2c.

@alexanderadam alexanderadam changed the title Explain why class << self is preferred over def self.foo Explain why def self.foo is preferred over class << self Sep 21, 2018
@alexanderadam
Copy link
Author

I'm sorry. I just recognized that the title of the issue was actually the inverse to what I wrote to in the description 🙈
I'm sorry in case that this was the cause of the misunderstanding here.

By the way, the particular text snipped I referenced here, is this sentence

Avoid class << self except when necessary, e.g. single accessors and aliased attributes.

@mmahalwy
Copy link

mmahalwy commented Feb 8, 2019

I agree. I think the cop should avoid having both class << self and def self.blah in the same file

@composerinteralia
Copy link
Member

RuboCop added a cop for this in 2020 that can be configured either way: rubocop/rubocop#8381. So I'd say this one is mostly personal preference, and our style guide now matches the default configuration in RuboCop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants