Proposed changes to tcrayford/Values

request.md

This gem is great. Thanks for releasing it!

We're running into 2 issues using Values, however.

1. Inheritance from other classes is impossible
2. No support for optional arguments to ValueClass.with

What I'm suggesting is a significant deviation from current usage and would require a major version release. That said, I think the benefits are worth it. Please review these ideas, and let me know if you would consider accepting these changes into the project, providing that the changes are simple and elegant enough to stay in line with the project's goal for simplicity. If not, we'll fork and release under a different name.

Inheritance

Rather than inheriting from Value.new, we think it would be more appropriate to follow the pattern I see in the Equalizer gem:

class GeoLocation
  include Value.new(:latitude, :longitude)
end

instead of

class GeoLocation < Value.new(:latitude, :longitude)
  # Cannot use Values and inherit from a different superclass
end

Optional Attributes

There are many valid use cases where a value object's attributes are optional. Being required to specify nil for these is frustrating and not elegant.

Consider these variants that would allow both of these:

Filter.with(matcher: /available/, limit: 100)
Filter.with(matcher: /available/)

• Consider all attributes optional, with no defaults:

  class Filter
    include Value.new(:matcher, :limit)
  end

• Explicitly mark attributes optional:

  class Filter
    include Value.new(:matcher, :limit)
    optional_attributes :limit # default to nil
    # or provide an explicit default
    optional_attributes limit: Float::INFINITY
    # or mark multiple attributes as optional
    optional_attributes matcher: //, limit: Float::INFINITY
  end

• Accept an options has in the constructor

  class Filter
    include Value.new(:matcher, :limit, defaults: {limit: Float::Infinity})
  end

• Don't support this feature in the gem; require users to override .with to provide defaults:

  class Filter
    include Value.new(:matcher, :limit)
  
    ATTRIBUTE_DEFAULTS = {limit: Float::Infinity}
  
    def self.with(hash)
      attributes = hash.dup
      attributes[:limit] ||= nil
      super(ATTRIBUTE_DEFAULTS.merge(hash))
    end
  end

@tcrayford Could you please comment on whether you like these changes and your opinion on the potential approaches?

/cc @josephjaber