Skip to content

Instantly share code, notes, and snippets.

@tomdalling
Created March 13, 2018 08:09
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save tomdalling/59afc9050ab459fb4a9787eb945c8aec to your computer and use it in GitHub Desktop.
Save tomdalling/59afc9050ab459fb4a9787eb945c8aec to your computer and use it in GitHub Desktop.
value object stuff

I like the idea of providing an RSpec shared example to test all these behaviours.

For the constructor, I would change "consider using keywords" to "should use keywords". Keywords should be the default, unless the order is really obvious.

Re: "Value construction options could be provided by keyword arguments." I would probably prefer value contruction options to be in separate class methods, instead of #new.

Re: caching. I would be hesitant to do this at the global level (e.g. stored in the class). If the cache never gets flushed, it could eat a lot of memory.

Re: freezing. This is good for basic types like strings/arrays/hashes, but I would not recommend freezing complicated types (i.e. custom classes).

Re: #merge. I like the name #merge, although I usually use #with.

Re: [each, of, structural, elements].map(&:hash).hash I don't think the .map(&:hash) is necessary. It could also be to_h.hash in most cases.

I went through the values gem and some of my own code to see if I could add anything, but I couldn't find anything extra. So I give these convetions a 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment