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.
- Inheritance from other classes is impossible
- 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/)
-
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 hash 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) 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