Dan refactor

[dankohn]

@david-a-wheeler

Apologies that this refactor changed most lines in project.rb. Major changes:

• Refactored from class methods to instance methods.
• Refactored passing? to use returns instead of a case/when for succinctness. Also changed to only require the criterion as a parameter.
• Removed use of sets since arrays are guaranteed unique and then frozen.
• Added monkey patches for Symbol#status and Symbol#justification. This DRYs up a lot of other methods.
• Alphabetized methods. Class methods come first because they must be defined before used, but should later be moved to a non-ActiveRecord Criteria model, as you indicate. Public and private methods are both alphabetized

Please enter comments below with questions or concerns.

[dankohn]

Also, removed badge_level_id? as it did not seem necessary (and it didn't just return a boolean, as the ? would indicate).

[dankohn]

Also improved regexp checks.

[david-a-wheeler]

Overall I love this refactoring. However:

1. The switch from sets to arrays has a performance downside. Every time we ask "is item X in Y" with an array it averages n/2 operations, aka O(n). If we use sets, it's O(1). We ask "is item X in Y" a lot. I could just merge, and put things back to sets if needed (it's really localized). Is there a reason we should avoid sets here?
2. I'm not a fan of monkey-patching - especially low-level built-ins like Symbol. Yes, I know Ruby allows it, and Rails actively uses it, but I fear that leads to subtle problems later. E.G., some other component's monkey-patching could easily lead to nasty problems if they overlap. Is there an easy and obvious alternative? Or is this really so conventional that my concerns are overblown?

Frankly, there's so much good here that I'll probably just merge in, and fix up things later if appropriate, but I'd really like to hear from you about those two points.

[dankohn]

1. Could you please show me one or more of the examples of where we're asking "is item X in Y"? My review is that we are far more likely to be iterating through all items, or that we're accessing them directly, in which case I presume that Ruby implements a hash table. In particular, please see the non-ActiveRecord Criteria model I implemented in the other pull request.
2. I think monkey patches are far less risky in a Rails app then in a gem, but I'm using your feedback to learn more about refinements, which are supposed to limit the patching. However, my initial implementation isn't working yet. http://interblah.net/why-is-nobody-using-refinements