Feature #6056
closedEnhancements to OpenStruct
Description
This patch fixes one issue, protecting #new_ostruct_member method, and possibly another by dup'ing marshal_dump, but I need more feedback on the later b/c I've also been told it is not needed.
The rest of this patch provides enhancements to OpenStruct that I feel are sorely needed, these include access via [] and []=, ability to mass update via merge!, minimal polymorphism with Hash and the addition of equality methods, eql? and ==.
https://github.com/ruby/ruby/pull/95
I'd also like opinions on further enhancements:
- adding #each and #each_pair
- making OpenStruct a subclass of BasicObject
Updated by marcandre (Marc-Andre Lafortune) over 12 years ago
- Assignee set to marcandre (Marc-Andre Lafortune)
- Target version set to 2.0.0
Hi.
In the future, please break requests in independent parts.
-
new_ostruct_member will be protected
-
==
is already defined. I don't think any coercion can be accepted in this case. You can't compare a Set and an Array directly, for instance. Open a new request if you disagree and show use case and how that would be consistent with other builtin and library classes. -
eql?
has already an issue opened: https://bugs.ruby-lang.org/issues/6029 . Please comment there if need be. -
each_pair
andto_h
is the subject of https://bugs.ruby-lang.org/issues/1400 . Please comment there if need be. -
marshal_dump
is not meant for public use, nodup
needed. I will remove the explicit documentation once issue 1400 is closed, as one should probably not rely on its implementation. -
"making OpenStruct a subclass of BasicObject". I don't see why and can see different problem with it. Please make a separate request for it with justification (e.g. why should it would be more helpful if it did not implement
respond_to?
,send
,object_id
, etc...) if you want this.
This leaves "Access via [] and []=", which we can discuss in this thread. I was wondering about this myself, about why it wasn't implemented like this to start with. I'm inclined to think it would be a good idea.
Updated by trans (Thomas Sawyer) over 12 years ago
Each change should be separate pull request or just a separate commit?
I knew about issue #6029, I was just trying to take care of that a couple of other features while I was involved with the code.
I see what you are saying about ==. I am actually surprised that:
class Q
def to_ary; [1,2,3]; end
end
Q.new == [1,2,3] #=> false
But that being the case, then okay I will remove.
I will also remove .dup from marshal_dump.
I will submit new issue about BasicObject, but briefly, the reason is for of use with #method_missing, to get as many methods out of the way as possible for it's use.
Updated by marcandre (Marc-Andre Lafortune) over 12 years ago
Hi,
Thomas Sawyer wrote:
Each change should be separate pull request or just a separate commit?
Ideally, both. It makes it easier to accept only some of the parts. As I said, "in the future"; no need to split your existing commits. In this case, the difficulty lies in deciding what should be done, not in implementing it.
Updated by trans (Thomas Sawyer) over 12 years ago
I've updated the pull request.
Updated by shyouhei (Shyouhei Urabe) over 12 years ago
- Status changed from Open to Assigned
Updated by ko1 (Koichi Sasada) about 12 years ago
ping.
status?
Updated by marcandre (Marc-Andre Lafortune) about 12 years ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r37376.
Thomas, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- lib/ostruct.rb: Add [] and []=, base on a patch by Thomas Sawyer
[ruby-core:42779] [Feature #6056]