dmendel / bindata

BinData - Reading and Writing Binary Data in Ruby
BSD 2-Clause "Simplified" License
577 stars 55 forks source link

parameters: default overrides explicit value if explicit value is false #97

Closed paddor closed 6 years ago

paddor commented 6 years ago

I have a default parameter that is true by default. I want to be able to set it to false on specific cases. Unfortunately, that does not work.

It seems that when setting the default values as fallback, what happens is something like:

@params[key] ||= default_for(key)

instead of:

@params[key] = default_for(key) if @params[key].nil?

Here's a demo:

require 'bindata'

class Foo < BinData::Record
  default_parameter use_bar: true

  uint8 :foo
  uint8 :bar, onlyif: ->{ use_bar.tap { |v| pp(use_bar: v) } } # print actual value
end

puts "testing DEFAULT (2 bytes) case:"
Foo.read("\x00\x00")
puts "GOOD"
puts

puts "testing with use_bar=true (2 bytes) case:"
Foo.read("\x00\x00", use_bar: true)
puts "GOOD"
puts

puts "testing with use_bar=false (1 byte) case:"
Foo.read("\x00", use_bar: false)
puts "GOOD"

BinData 2.4.1 on Ruby 2.5.0 prints:

testing DEFAULT (2 bytes) case:
{:use_bar=>true}
GOOD

testing with use_bar=true (2 bytes) case:
{:use_bar=>true}
GOOD

testing with use_bar=false (1 byte) case:
{:use_bar=>true}
Traceback (most recent call last):
    11: from bindata_bug_2.rb:24:in `<main>'
    10: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/base.rb:21:in `read'
     9: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/base.rb:145:in `read'
     8: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/base.rb:254:in `start_read'
     7: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/base.rb:147:in `block in read'
     6: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/struct.rb:139:in `do_read'
     5: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/struct.rb:139:in `each'
     4: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/struct.rb:139:in `block in do_read'
     3: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/base_primitive.rb:129:in `do_read'
     2: from (eval):23:in `read_and_return_value'
     1: from /home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/io.rb:276:in `readbytes'
/home/p/.gem/ruby/2.5.0/gems/bindata-2.4.1/lib/bindata/io.rb:314:in `read': End of file reached (EOFError)

Note that in the 3rd test case, use_bar should have been false instead of true.

paddor commented 6 years ago

This seems to work just fine if I use virtual :use_bar, initial_value: true instead of a default_parameter. If I just used the library in the wrong way, you can close this issue. Thank you.

dmendel commented 6 years ago

virtual :use_bar, initial_value: true may or may not work.

BinData uses nil values for signalling and as nil and false are both falsey, there may be some unexpected interaction. I'll have to examine the code to be sure.

I recommend you use ints instead.

class Foo < BinData::Record
  default_parameter use_bar: 1

  uint8 :foo
  uint8 :bar, onlyif: ->{ use_bar == 1 }
end

Foo.read("\x00\x00", use_bar: 1)
Foo.read("\x00\x00", use_bar: 0)
paddor commented 6 years ago

Yeah, for now I'm using ints. It's just not very Ruby-like to use an int instead of true/false. If you think it's possible to adapt BinData, that would be awesome. Otherwise, no big issue. Thanks anyway!

dmendel commented 6 years ago

Implemented in 2.4.2.

Your original example will now work as expected.

paddor commented 6 years ago

Thanks a lot! That's awesome! :smile: