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

X = Struct.new(:x) do is bad #744

Open
iGEL opened this issue Apr 25, 2019 · 1 comment
Open

X = Struct.new(:x) do is bad #744

iGEL opened this issue Apr 25, 2019 · 1 comment
Labels

Comments

@iGEL
Copy link

iGEL commented Apr 25, 2019

The style guide suggest to use

X = Struct.new(:x) do
  def method
  end
end

# instead of

class Y < Struct.new(:y)
  def method
  end
end

I think this is a bad recommendation. Yes, the later creates an unnecessary anonymous class, but it uses the usual syntax to create classes and thus is easy to understand. Even Rubocop doesn't recognize the assignment syntax everywhere (E.g. Style/Documentation doesn't care about this).

Also, this leads to hard to understand errors:

X = Struct.new(:x) do
  XX = true
end

defined?(X::XX) # => nil
defined?(XX) # => "constant"

class Y < Struct.new(:y)
  YY = true
end

defined?(Y::YY) # => "constant"
defined?(YY) # => nil

This is possible, but is it better?

Z = Struct.new(:z)
class Z
  ZZ = true
end

defined?(Z::ZZ) # => "constant"
defined?(ZZ) # => nil
@bbatsov
Copy link
Collaborator

bbatsov commented Jun 12, 2019

Great points! It's pretty similar to the problem with X = Class.new, which many tools won't understand as a class definition.

Even Rubocop doesn't recognize the assignment syntax everywhere (E.g. Style/Documentation doesn't care about this)

Fixing this in RuboCop shouldn't be hard, btw. That being said we can probably relax the guide and add more details to that particular rule.

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

No branches or pull requests

2 participants