Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Built-in boolean support #239

Open
maxlinc opened this issue Nov 12, 2014 · 6 comments
Open

Built-in boolean support #239

maxlinc opened this issue Nov 12, 2014 · 6 comments

Comments

@maxlinc
Copy link
Contributor

maxlinc commented Nov 12, 2014

When I wrote the support for coercing primitive types, I wrote:

Hashie does not have built-in support for coercion boolean values, since Ruby does not have a built-in boolean type or standard method for to a boolean. You can coerce to booleans using a custom proc.

Although that's true there is a boolean standard for json and a slightly less strict standard for yaml. I think those should be common enough that it might make sense to ship an implementation that handles validation/coercion for one or both standards.

So rather than the proc currently in the README:

class Tweet < Hash
  include Hashie::Extensions::Coercion
  coerce_key :retweeted, ->(v) do
    case v
    when String
      return !!(v =~ /^(true|t|yes|y|1)$/i)
    when Numeric
      return !v.to_i.zero?
    else
      return v == true
    end
  end
end

You could do:

class Tweet < Hash
  include Hashie::Extensions::Coercion
  coerce_key :retweeted, Hashie::Boolean # or perhaps Hashie::JSON::Boolean or Hashie::YAML::Boolean
end

This isn't equivalent to the sample proc, but its similar and more standard. For example, the YAML standard doesn't coerce numerics.

@dblock
Copy link
Member

dblock commented Nov 15, 2014

I would be down with this.

@mkcode
Copy link

mkcode commented Oct 14, 2016

I did this in my application, leveraging ActiveModel:

class ProfileData < Hashie::Dash
  include Hashie::Extensions::Dash::Coercion

  class Boolean
    def self.coerce(value)
      ::ActiveModel::Type::Boolean.new.cast(value)
    end
  end

  property :is_admin, coerce: Boolean

  ...
end

Although this is actually a non-trivial issue. If you reference different libraries, you will find varied behavior.

See ActiveModel:

class Boolean < Value
  FALSE_VALUES = [false, 0, "0", "f", "F", "false", "FALSE", "off", "OFF"].to_set

  def cast_value(value)
    if value == ""
      nil
    else
      !FALSE_VALUES.include?(value)
    end
  end

Defined here: https://github.com/rails/rails/blob/159b774887cfb3d5c862b2b48d24d75b658e47af/activemodel/lib/active_model/type/boolean.rb

Now check out dry-types - another very well done type library:

module Dry
  module Types
    module Coercions
      module Form
        TRUE_VALUES = %w[1 on On ON t true True TRUE T y yes Yes YES].freeze
        FALSE_VALUES = %w[0 off Off OFF f false False FALSE F n no No NO].freeze
        BOOLEAN_MAP = ::Hash[TRUE_VALUES.product([true]) + FALSE_VALUES.product([false])].freeze
...

Defined here: https://github.com/dry-rb/dry-types/blob/6fa7aff1257cddc2e46e3f022db4d2fa6bde318f/lib/dry/types/coercions/form.rb#L8-L10

Very different behavior between these two. ActiveModel returns nil only for empty string, false for the above FALSE_VALUES and true for EVERYTHING else. Dry-types returns true and false for the strings with the TRUE/FALSE_VALUES, and nil for everything else.

If this was up to me - we would totally redo to coercion system, with a thin wrapper to provide compatibility with the type library of your choosing. ActiveModel::Type and Dry::Types would be a great start. Note that Dry::Types gets you far more fine grained control over your types: http://dry-rb.org/gems/dry-types/built-in-types/

@dblock
Copy link
Member

dblock commented Oct 14, 2016

@mkcode That makes sense and is certainly ambitious, but who dares wins!

@mkcode
Copy link

mkcode commented Oct 17, 2016

@dblock - Are you the maintainer for this project? I may take a stab at creating a PR for this - I'd love to be able to use Dry::Type with Hashie since I use it elsewhere in one one of my applications. I would want to get your input on how to implement this, or what you think the API should be in order to enable this.

For example:

We could switch the type system based off the availability of gems with a predefined priority. Like use Dry::Types if it is available, otherwise use ActiveModel::Type if it is available, or otherwise use the current builtin coercion. This may break backwards compatibility if someone upgrades though.

Or we could create a class level option?

class ProfileData < Hashie::Dash
  coercion_class :active_model
  include Hashie::Extensions::Dash::Coercion
  ...
end

Explicit module names?:

class ProfileData < Hashie::Dash
  include Hashie::Extensions::Dash::Coercion::ActiveModel
  ...
end

I think I would prefer an option to the included coercion module, like so:

class ProfileData < Hashie::Dash
  include Hashie::Extensions::Dash::Coercion(:active_model)
  ...
end

Please instruct!

@dblock
Copy link
Member

dblock commented Oct 17, 2016

@mkcode I am. One of. Trying to. Along with some other lovely people. I have the glorious merge/release privileges ;)

@dblock
Copy link
Member

dblock commented Oct 17, 2016

I prefer the very explicit method that defaults to the old way, aka exactly what you said @mkcode. Give it a try!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants