Proposed addition of value class Enum::Value by ginjo · Pull Request #7 · mezuka/enum

ginjo · 2017-12-07T07:04:44Z

I created a class Enum::Value to spawn instances of immutable values associated with Enum::Base subclasses. I'm presenting this as a PR, in case you think it fits with the safe-enum gem. I'm open to all feedback: Does it mesh well with your idea of what the safe-enum gem is all about? Is it implemented in the best way? Is it redundant or misguided?

Background: I've been working on a project using rom-rb and dry-types. The backend database can be variable (sql, nosql, other...), so I'd like to keep my enum implementation in pure ruby. I chose the safe-enum gem as the best starting point to build a Value class on top of. It works perfectly for my project, and it's simple enough that it might be helpful to others.

If you think it is worthy of merging, I'm happy to help get it there. If not, no worries.

I have some preliminary documentation here: https://gist.github.com/ginjo/04adffb4b39f13d20645b91205a8f683

And I can explain more about the architecture and coding decisions, if you're interested.

Thanks for the great work in safe-enum!
William

Change default settings of Enum::Value subclass.

ka8725 · 2017-12-07T12:48:50Z

+    # Load a primitive (symbol, string, integer) into new enum Value instance,
+    # taking into consideration enum constraints, default_value setting.
+    # Returns frozen Value instance.
+    def initialize(new_val) #, enum_class)


remove commented code

I agree, raw_val is a better name for that variable. I'll change that and clean up the comments.

ka8725 · 2017-12-07T12:53:09Z

+      #   @default_value => <:ERROR|:ANY|:something|nil>
+      #   @suppress_read_errors => <true|false>
+      subclass.suppress_read_errors = false
+      subclass.default_value = :ERROR


It looks strange that default value is ERROR, why not ANY? 🤔
Also I don't like capitalized versions... May be _error, $error and etc. It needs to be elaborated more. We need to come up with a name that potentially won't conflict with user input.

On the spectrum between safe and convenient, I went with safe, mostly because the title of the gem is safe-enum 😄 . The default behavior will raise an exception whenever the Value object is loaded or read with an invalid token. There are goods and bads about either of the 'special' default settings.

With a safe default :ERROR, users are more likely to raise exceptions that stem from data issues, especially if using this to model data pulled from a database.

With a relaxed default :ANY, users will get value objects without exceptions, though the objects may be invalid. If this is the default for default_value, should suppress_read_errors also default to true?

A possible middle ground is a default value of nil. The user won't accidentally use an invalid value, yet they won't be interrupted by exceptions when loading data from a db. They can still check for valid? and look at @error to get more information.

Your suggestions for alternatives to the special defaults are fine. :$error is interesting, I haven't considered $ in a symbol name before. Probably much less chance of clashing with user input.

Ok, it's clear. But I'm still for renaming capitalized words to lowercase (I mean :$error).

No problem, I'll rename those per your suggestions.

ka8725 · 2017-12-07T13:07:33Z

@ginjo thank you for you interest in this gem and the pull request you made. Your proposal looks promising, but it's not clear what problem does it solve. Could you please specify problems that you had in your project with some code examples, so that it will be clear if the solution is good enough or not. Thanks!

ginjo · 2017-12-08T07:00:28Z

@ka8725 Thanks for reviewing and for the feedback!

I created the Enum::Value class to serve two common use cases (well, common for me at least... please see below). I realize these could be handled with functional technique using just the current Enum::Base class. So I guess it comes down to providing an OO approach to handling enum values, and for me that keeps my code dryer.

I'll address your code comments separately (good points, btw). Meanwhile, here are some examples.

As value objects loaded into entity models built from database records.

class UserStatuses << Enum::Base
  values :none, :new, :registered, :active, :admin
  default_value nil
end

# UserStatuses is used when building entity models from db records.

user = UsersRepo.get(id)
# => #<ROM::Struct::User id=1 username="billy" email="bill@abc.com" ... >

user.status
# => #<UserStatuses::Value:0x007fb4137df8d8 @stored_value=:active>
    
case
  when user.status >= :registered
    # do stuff
  when user.status == :new
    # do other stuff
  when user.status < :new
    # ...
end

As a singular point of query for calculated status on a complex process.

class ReactorStatuses < Enum::Base
  values :blue, :aqua, :green, :yellow, :orange, :red
end

# A bunch of calculations to get real-time process status here, until finally...
# => core_reactor_report

core_reactor_report.status
# => #<ReactorStatuses::Value:0x007fb41367c540 @stored_value=:orange>

case (s = core_reactor_report.status)
when s <= :blue
  # do nothing
when s.between? :aqua, :yellow
  # notify ops
when s > :yellow
  # all hands on deck
end

Clean up comments. Test coverage for all instance & class methods of Enum::Value.

ka8725 · 2017-12-09T20:48:31Z

+        end
+      end
+      self.freeze
+      self


no need to return self from an initializer.

Oh right... I probably thought I was working on .new 😅. Will fix.

ka8725 · 2017-12-09T20:52:28Z

Sorry @ginjo currently I don't have time to look into this deeply. I will promise will do this soon. I would like also to make some cosmetic changes in the proposed code to not bother you a lot with that boring things. Thanks.

ka8725 · 2017-12-09T21:10:55Z

+module Enum
+  class Value
+
+    # TODO: Perhaps stored_value should be renamed unsafe_value?


Let's call it atom, wdyt? The idea borrow from this.

Ok, I like atom.

ginjo · 2017-12-10T06:10:45Z

No worries, take your time. Thanks again for considering the pr.

Fixed code that was producing ambiguous argument warnings.

which makes it easier to run individual tests: bundle exec ruby -I./ test/value_test.rb Added require 'enum' to value.rb.

ginjo added 5 commits December 5, 2017 19:59

Add Value class to create immutable instances of enum values.

b7f529b

Fix delegation bug in init_child_class.

184b930

Add value_test.rb and several specs.

b83bd99

Add tests for Value instance methods.

310c762

Change .default to .default_value.

f4cb96d

Change default settings of Enum::Value subclass.

ginjo changed the title ~~Valueclass~~ Proposed addition of value class Enum::Value Dec 7, 2017

ka8725 reviewed Dec 7, 2017

View reviewed changes

Update test labels.

566832f

Change new_val to raw_val in Enum::Value#initialize.

c19dae2

Clean up comments. Test coverage for all instance & class methods of Enum::Value.

ka8725 reviewed Dec 9, 2017

View reviewed changes

ginjo and others added 4 commits May 17, 2025 20:09

Changed :ERROR and :ANY to :$error and :$any

ccec43c

Fixed code that was producing method redefinition warnings.

41d96c0

Fixed code that was producing ambiguous argument warnings.

Updated test files to make use of require_relative,

81bb949

which makes it easier to run individual tests: bundle exec ruby -I./ test/value_test.rb Added require 'enum' to value.rb.

Removed require 'enum' to value.rb... it was circular.

5d23f9b

Conversation

ginjo commented Dec 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ka8725 commented Dec 7, 2017

Uh oh!

ginjo commented Dec 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

As value objects loaded into entity models built from database records.

As a singular point of query for calculated status on a complex process.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ka8725 commented Dec 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ginjo commented Dec 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ginjo commented Dec 8, 2017 •

edited

Loading