tcrayford/Values

Inheritance and Optional Attributes

nilbus opened this issue · 4 comments

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)
        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

@ms-ati You've contributed a lot here, so I'd love to have your feedback on this too.

@nilbus I have no plans to do anything like this in Values - it's intentionally small and not very featureful.

+1 I would love to see this feature.

I'd recommend writing your own gem for these needs. Values is intentionally small