Project

General

Profile

Actions

Feature #6056

closed

Enhancements to OpenStruct

Added by trans (Thomas Sawyer) about 10 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Normal
Target version:
[ruby-core:42779]

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

Related issues 2 (0 open2 closed)

Related to Ruby master - Feature #1400: Please add a method to enumerate fields in OpenStructClosedmarcandre (Marc-Andre Lafortune)04/23/2009Actions
Related to Ruby master - Bug #6029: Should OpenStruct implement #eql? and #hash?Closedmarcandre (Marc-Andre Lafortune)02/15/2012Actions

Updated by marcandre (Marc-Andre Lafortune) about 10 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 and to_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, no dup 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) about 10 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) about 10 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) about 10 years ago

I've updated the pull request.

Actions #5

Updated by shyouhei (Shyouhei Urabe) about 10 years ago

  • Status changed from Open to Assigned

Updated by ko1 (Koichi Sasada) over 9 years ago

ping.
status?

Actions #7

Updated by marcandre (Marc-Andre Lafortune) over 9 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.


Actions

Also available in: Atom PDF