ObjectSet #2

wants to merge 2 commits into


None yet
2 participants

refset commented Mar 25, 2013

Hopefully not too out-of-scope!


PeterScott commented Mar 27, 2013

It's an interesting change, but it has a few problems:

  1. The indentation is messed up.
  2. You committed some of the emacs-style "filename~" temporary files, which don't belong here.
  3. Probably a better way to do it would be to abstract out the common parts of StringSet and ObjectSet into something that takes a key function (let's call it StringKeySet). If we had that, defining StringSet and ObjectSet in terms of this base object could be done in, like, two lines.

I have kind of forgotten about this repo (and forgotten how to release things onto npm), but if you wanted to fork the project, making a generic StringKeySet would be a clear win over my current code.

PeterScott closed this Mar 27, 2013

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