[ruby-core:120293] [Ruby master Bug#20943] Constant defined in `Data.define` block

["zverok (Victor Shepelev) via ruby-core" <ruby-core@ml.ruby-lang.org>]

Issue #20943 has been updated by zverok (Victor Shepelev).


TBH, for bigger `Struct`/`Data`-based classes I typically prefer a regular inheritance instead of using class definition block—both for just aestetics (“it looks like a class definition”) and to avoid behavioral differences like constant definition discussed here, or being able to treat `Data` like a proper base class:

For example, `.new` for `Data`-based classes accepts both positional and keyword arguments. But if one is _sure_ they want to change this behavior, with regular inheritance, it is extremely easy:
```ruby
class Unit < Data.define(:value)
  def self.new(value, **nil) = super(value:)
end

Unit.new(1) #=> #<data Unit value=1>
Unit.new(value: 1)
# no keywords accepted (ArgumentError)
```
Such easy redefinition (using `super`) is not available with `Data.define(...) { ... }`.

For all I know, there are two counter-arguments for regular inheritance for those cases:
1. Creation of unnecessary anonymous immediate classes
2. Problems with code reloading

In my dayjob (large regular Rails applications) I find the former negligible, and never met with the latter, but this experience might not be universal.

Yet theoretically, I’d rather thought in the direction of changes that would make regular inheritance from `Data.define`/`Struct.new` less problematic (if it really is, and it is not just “how we are used to think of it”). 

Just to notice here: Other than Struct/Data, there are other libraries that make use of “inheriting of dynamically produced classes” approach (though I didn’t look into the implementation details for many years, so I am not sure how it is implemented currently), like [Sequel](https://sequel.jeremyevans.net/rdoc/files/README_rdoc.html#label-Sequel+Models):
```ruby
class Post < Sequel::Model(:my_posts); end
```
or, IIRC, some of the dry-rb gems.

So, it might be, that “optimizing of inheritance from dynamic classes/modules” is more desirable decision than complication of block scopes.

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111058

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/