Ruby - Centroid::Config vs Hash inadequacies

[thorncp]

If a method uses a hash to handle optional arguments, we cannot currently give it a Centroid::Config instance and expect it to work. The case that we ran into is as follows, but there are more if we want to go into them.

require 'centroid'
config = Centroid::Config.new('{}')

def doit(properties = {})
  properties["a_key"] ||= "default value"
  p properties
rescue => e
  p e.message
end

doit # => {"a_key"=>"default value"}
doit({}) # => {"a_key"=>"default value"}
doit(config) # => "Centroid::Config instance does not contain key: a_key"

One workaround on the calling side is to have the method be aware that Centroid exists, and perform checks to determine if it's dealing with a Centroid::Config instance. This could also do is_a?(Hash), is_a?(Centroid::Config), etc., but it will always have the knowledge that Centroid exists (by either Centroid::Config or raw_config).

require 'centroid'
config = Centroid::Config.new('{}')

def doit(properties = {})
  properties = properties.raw_config if properties.respond_to?(:raw_config)
  properties["a_key"] ||= "default value"
  p properties
end

doit # => {"a_key"=>"default value"}
doit({}) # => {"a_key"=>"default value"}
doit(config) # => {"a_key"=>"default value"}

A workaround on the library side could be to start implementing Hash methods to give the appearance of actually being a Hash. (this is not a good idea)

I think a better idea would be to implement the to_h method that simply returns a hash representation, and calling code can rely on that. This moves the code away from knowing anything about Centroid, and just expects you to provide something that can become a Hash.

require 'centroid'
config = Centroid::Config.new('{}')

class Centroid::Config
  def to_h
    raw_config.dup
  end
end

def doit(properties = {})
  properties = properties.to_h # noop for Hash instances
  properties["a_key"] ||= "default value"
  p properties
end

doit # => {"a_key"=>"default value"}
doit({}) # => {"a_key"=>"default value"}
doit(config) # => {"a_key"=>"default value"}

[jtroe]

So if we went that direction would we want to duplicate that method on the other environments? I think the behavior on the other environments is more consistent with what we want, but this would make inconsistent implementations for the different environments.

Not sure if that makes any sense.

[thorncp]

I'm not convinced this would make inconsistent implementations. The functionality already exists via raw_config, but the use of Hashes is much more ingrained in Ruby than C# (dunno about Python), and to_h is the idiomatic way to make the conversion.

However, I do think it would make sense to provide analogous methods for C# (ToDictionary()?) and Python (no idea).

[thorncp]

I'll also refer to #43 and suggest these methods return new objects, so the underlying config cannot be mutated.