Fallback value to OpenStruct delete_field

[jfrazx]

Provide a value to use in the event a field does not exist in your OpenStruct.

require 'ostruct' 

person = OpenStruct.new('name' => 'John Smith', 'age' => 70)

person.delete_field('number') { 8675_309 } 
# => 8675309

[marcandre]

I'm not against it, but I'm wondering if there's a particular use case (compared to delete_field('foo') || value)?

[marcandre]

With a bit more thought, given the nature of OpenStruct, I'm not sure that it's a good idea to make a difference between a member being defined and nil and one not being defined. In particular, there is no method fetch on OpenStruct, so I really wonder why delete_field would give access to that distinction.

[jfrazx]

delete_field('foo') || value won't work. The current handling of a missing field is to rescue NameError and then re-raise NameError with a custom message. To recover you would also have to rescue. This, seems to me, provides a cleaner solution, if desired.

[marcandre]

Oh, right, my bad. So I guess the equivalent would be

result = person['number'] ||= 8675_309
person.delete_field 'number'

Still, the question remains, what is the use case?

[jfrazx]

I currently use it in conjunction with optparse.

OptionParser does not have a mechanism for required parameter.

opts.on("-r", "--required", "This is required with no way to enforce it") { |o| options.required = o }

So, what I might currently have to do is this:

begin 
  required = options.delete_field(:required)
rescue NameError 
  raise MyCustomErrorClass, "You failed to supply required information"
end 

Since the NameError has already been rescued in the delete_field method, by supplying a block I can skip rescuing OpenStructs customized NameError and just raise my own.

required = options.delete_field(:required) do 
  raise MyCustomErrorClass, "You failed to supply required information"
end

Also, if there is no field set for some information that you can supply a basic value for, this:

foo = options.delete_field(:foo) { 'some foo value' }

is much more convenient than this:

foo = if options.respond_to?(:foo) 
            options.delete_field(:foo)
          else
            'some foo value'
          end 

or

foo = options.foo ||= 'some foo value' 
options.delete_field(:foo)

I should point out that if a field is set, this change will not affect it, even if the value for the field is nil.

person = OpenStruct.new('name' => 'John Smith', 'age' => 70, 'address' => nil)

address = person.delete_field(:address) { '1600 Pennsylvania Ave' }
# => nil 

So it is not equivalent to the example above, nil might be an acceptable result.

address = person['address'] ||= '1600 Pennsylvania Ave'
# => '1600 Pennsylvania Ave' 

Obviously there are already ways to deal with the situations I am presenting, but, I feel, with this change it provides a simpler, more efficient means to achieve that result with no negative impact.

[marcandre]

Thanks for the examples. Why do you need to call delete_field? The first example could simply be

options[:required] ||  raise MyCustomErrorClass, "You failed to supply required information"

And why is options an OpenStruct instead of a Hash?

[jfrazx]

I'm using an OpenStruct instead of a Hash because I prefer the dot notation.

options.thing   = true 
options[:thing] = true

I need to use delete_field because if the information does exist it needs to be removed from the structure. Eventually it will be turned into a Hash and shoved into a database.

required = options.delete_field(:required) do 
  raise MyCustomErrorClass, "You failed to supply required information"
end

things = options.delete_field(:thing) { 'value' } 

options.table_data ||= "some table data"

# delete more fields/manipulate data...

my_database_thing = MyDatabaseThing.create(options.to_h)

I initially expected OpenStruct to behave more like a Hash.

hash_options = Hash.new

required = hash_options.delete(:required) do 
  raise MyCustomErrorClass, "You failed to supply required information"
end
# => MyCustomErrorClass: You failed to supply required information

# OpenStruct current implementation
required = options.delete_field(:required) do 
  raise MyCustomErrorClass, "You failed to supply required information"
end
# => NameError: no field `required' in #<OpenStruct>


foo = hash_options.delete(:foo) { 'some foo value' }
# => "some foo value"

Arrays behave in this same fashion.

a = Array.new 

foo = a.delete(:foo) { 'some foo value' }
# => "some foo value"

foo = a.delete(:foo) do |value|
  raise MyCustomErrorClass, "no such value #{value} in array"
end 
# => MyCustomErrorClass: no such value foo in array

Thus, the proposed change brings OpenStruct more inline with what is common for other basic Ruby data structures. Taking it a step further and not raising a NameError and returning nil would make it even more uniform.

[k0kubun]

Now master branch has a required check "check_branch", but I couldn't update your branch to include it. Could you rebase this from master to run it?

[k0kubun]

As we cannot merge this anyway without passing the check_branch, could you reopen this after rebasing this from the master branch? Thanks.

[jfrazx]

@k0kubun Rebased

[marcandre]

I'm 👍 on this, after the small comments have been addressed.

[jfrazx]

@marcandre I believe this fits all the requirements now.

[marcandre]

Looks good. A small comment and this is ready to merge.

[marcandre]

Thanks @jfrazx

Merged.

Notes:

1. Consider setting your text editor to remove trailing whitespaces
2. Methods like block_given? should not be used directly within OpenStruct. I'm fixing this in this commit

Will make a release soon.