Project

General

Profile

Actions

Feature #15815

open

Add option to raise NoMethodError for OpenStruct

Added by mtsmfm (Fumiaki Matsushima) over 5 years ago. Updated 8 months ago.

Status:
Assigned
Target version:
-
[ruby-core:92513]

Description

GitHub PR: https://github.com/ruby/ruby/pull/2164

Currently, OpenStruct#method_missing returns nil even if the key isn't registered.

require 'ostruct'
os = OpenStruct.new({a: 1})
os.a #=> 1
os.b #=> nil

I'd like to add exception option to raise NoMethodError in such case.

require 'ostruct'
os = OpenStruct.new({a: 1}, exception: true)
os.a #=> 1
os.b #=> NoMethodError

Use case

I sometimes use OpenStruct as a JSON API response wrapper.
It's useful to use method call instead of key access (obj[:key]) because we can use Symbol#to_proc if it's a method (for example users.map(&:id))

But I want to prevent typo for a key name. Currently users.map(&:idd) just returns [nil,...]

Even if we have this exception option, we can't enable this option for JSON parser easily though:

JSON.parse(response, object_class: Class.new(OpenStruct) { def initialize(hash); super(hash, exception: true); end })

What do you think?


I've searched with "openstruct nomethoderror" on bugs.ruby-lang.org though, please let me know if it's duplicated.
https://bugs.ruby-lang.org/search?utf8=%E2%9C%93&scope=&q=nomethoderror+openstruct

Updated by marcandre (Marc-Andre Lafortune) over 5 years ago

It's hard to decide on features of OpenStruct as it's somewhat an antipattern.

Note that you might be able to write users.map{@[:id]} in the next version and avoid using an OpenStruct altogether.

That being said, I am wondering if a frozen OpenStruct could possibly raise an error upon reading an undefined key.

Updated by shevegen (Robert A. Heiler) over 5 years ago

Note that you might be able to write users.map{@[:id]} in the next version and
avoid using an OpenStruct altogether.

I think that the idiom, or "mental mode", e. g. how users use ruby, is a bit different
between the two examples. At the least to me, "OpenStruct" is instantly recognizable
and "exception: true" is also no longer a rare idiom, after some core methods accepted
it; the map {@[:id] variant, at the least to me, is slightly harder to understand for
my brain. But anyway, that's just my opinion - to the suggestion itself, one described
use case:

But I want to prevent typo for a key name.

Although I personally do not use struct and openstruct a lot, even though I think it is
a cool idea (prototypical objects all the way, at all times), to me the suggestion appears
useful. So I am in light support of the suggestion; I don't feel particularly strong
either way, though. (May help for people to comment who actually use struct and openstruct
a lot.)

Updated by ko1 (Koichi Sasada) over 5 years ago

mtsmfm-san, if you are interest about this ticket yet, could you file on our dev-meeting agenda?
https://bugs.ruby-lang.org/issues/15996

Thanks.

Updated by osyo (manga osyo) over 5 years ago

What about adding block arguments to OpenStruct.new like Hash instead of options?

h = Hash.new {|hash, key|
  raise(IndexError, "hash[#{key}] has no value")
}

# Error: in `block in <main>': hash[1] has no value (IndexError)
h[1]


require "ostruct"

# Adding block arguments like Hash
os = OpenStruct.new({a: 1}) {|open_struct, method_name|
  raise(NoMethdError, "undefined method `#{method_name}' for #{open_struct}")
}

# Error: in `block in <main>': undefined method `b' for #<OpenStruct a=1> (NoMethodError)
os.b

Updated by esparta (Espartaco Palma) about 5 years ago

Personally I found the proposed signature confusing, more when taking in account that OpenStruct calls:

os = OpenStruct.new(a: 1, exception: true)

# vs

os = OpenStruct.new({a: 1}, exception: true)

I'd like to add exception option to raise NoMethodError in such case.

require 'ostruct'
os = OpenStruct.new({a: 1}, exception: true)
os.a #=> 1
os.b #=> NoMethodError

How about a subclass of OpenStruct to take this feature?

class ExceptionalOpenStruct < OpenStruct
   def method_missing(mid, *args)
      if args.length == 0
         raise NoMethodError unless @table.key?(mid)
        @table[mid]
      else
         super
      end
   end
end

eostr = ExceptionalOpenStruct.new(a: 1)
eostr.a 
# => 1
eostr.b
# NoMethodError: NoMethodError
ebstr.b = 2
eostr.b
# => 2

(I'm probably missing some feature, but that's the idea...)

Another concern is probably the performance: OpenStruct will check another branch just to verify if now it also need to raise an exception.
The flexibility of OpenStruct makes it not exactly fast, handling exception would make all other uses cases more slow performance wise.[*benchmark needed]

osyo (manga osyo) wrote:

What about adding block arguments to OpenStruct.new like Hash instead of options?

h = Hash.new {|hash, key|
  raise(IndexError, "hash[#{key}] has no value")
}

# Error: in `block in <main>': hash[1] has no value (IndexError)
h[1]


require "ostruct"

# Adding block arguments like Hash
os = OpenStruct.new({a: 1}) {|open_struct, method_name|
  raise(NoMethdError, "undefined method `#{method_name}' for #{open_struct}")
}

# Error: in `block in <main>': undefined method `b' for #<OpenStruct a=1> (NoMethodError)
os.b

Updated by matz (Yukihiro Matsumoto) about 5 years ago

I like the OP's idea. It's up to @marcandre (Marc-Andre Lafortune)

Matz.

Updated by marcandre (Marc-Andre Lafortune) about 5 years ago

  • Assignee set to marcandre (Marc-Andre Lafortune)

Updated by marcandre (Marc-Andre Lafortune) about 4 years ago

I'm proposing to add OpenStruct::Strict, that will not return nil for unknown attributes and instead raise a NotMethodError. See PR https://github.com/ruby/ruby/pull/3594 (last commit)

Usage with JSON is easy:

JSON.parse(response, object_class: OpenStruct::Strict)

This seems to me to avoid issues that other solutions had:

  • Any other solutions is difficult to use with JSON.parse API
  • I considered adding OpenStruct#strict! (returning self after mutating it), seems the 2nd best option.
  • OpenStruct.new({...}, exception: true) made it difficult to remain compatible with OpenStruct.new(using: 'keyword arguments'). Also seemed confusing API.
  • OpenStruct.new(...) { block to handle unknown attribute } was longer to write, and potentially less/not Ractor compatible, and not serializable in general.

The downside to OpenStruct::Strict is that it makes adding any other configuration more difficult. Given the fact that OpenStruct is to mature now though, I feel it is not an actual issue. Anyone wanting to subclass has to pick a base class (or define it's own respond_to_unknown_attribute!).

Comments welcome.

Updated by mtsmfm (Fumiaki Matsushima) about 4 years ago

Cool.
It's much better API to integrate with JSON.parse.

Actions #11

Updated by hsbt (Hiroshi SHIBATA) 8 months ago

  • Status changed from Open to Assigned
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0