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 future Pull Requests for 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.

Given the use case below, consider the 4 following approaches to allow this:

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

1. Consider all attributes optional, with no defaults:

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

2. 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

3. Accept an options hash in the constructor

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

4. 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