Refactoring in transforms.rb

[pjotrp]

I have a problem reading this code. I presume symbols like :round are JSON tags and you are setting properties after validation. Is there no more elegant and clear way that this code can work?

    def treemap(args)
      valid = args[:value] && (args.keys - [:padding, :ration, :round, :size,
                                            :sticky, :value]).empty?
      valid &&= args[:value].is_a?(String)
      valid &&= args[:padding].nil? || args[:padding].is_a?(Numeric) ||
          array_of_Numeric?(args[:padding], size=4)
      valid &&= args[:ratio].nil? || args[:ratio].is_a?(Numeric)
      valid &&= args[:round].nil? || [true, false].include?(args[:round])
      valid &&= args[:size].nil? || array_of_Numeric?(args[:size], size=2)
      valid &&= args[:sticky].nil? || [true, false].include?(args[:sticky])
      if valid
        set_properties(args)
      else
        raise ::Plotrb::InvalidInputError
      end
    end

Maybe you should be using a proper parser/lexer to do this stuff. I mean, if Vega is much larger, this is going to be problematic.

[clbustos]

Maybe, args hash should be replaced with specific Structs. The Struct could be responsible of verification of parameters, so you could separate the logic of the method from the parameter checking.

[translunar]

Ahh! This is a common issue. "When is it appropriate to check arguments and when not?"

Don't check arguments when:

• Another method called downstream will do the checking for you

Do check arguments when:

• The error message from the downstream method is vague, or
• There is confusing misbehavior without an error message (rare that misbehaviors are not confusing)

I think generally the appropriate exception to raise is ArgumentError. Sometimes it's something else, like a FileError. Try to find similar examples elsewhere in Ruby code, especially ruby-core.

[zuhao]

A few rounds of refactoring have been done and this issue can be closed.