enspiral/loomio — pull request #14 by craigambrose (2012-03-07 02:18:13+00:00)
Title: Some refactoring of model layer, including adding Acts as State Machine

The first commit in this pull request has a lot of small refactoring changes to the model layer. I went through all the model classes and their tests, and mainly touched up little things that I thought were un-ruby-ish. These are pretty subjective of course. I'm not normally one to go in and monkey with someone else's code on such semantic issues, but I thought that it might be interesting to do so here purely to demonstrate the rubyish way of doing things. Some of this codebase bears the hallmarks of being written by someone coming from a Java-like language but fairly new to ruby. Stuff like unnecessary calls to self, use of the "return" keyword on the last line of a method, unnecessary parenthesis in conditionals is all quite allowed in ruby, but you typically wont find ruby programmers doing it. The goal is to shoot for something as free from unnecessary syntax elements as possible, except where you can't. I realise this leads to inconsistance. For example, you don't need to use "self", except where you do :) , which is whenever you're assigning to a instance member of the class (because ruby can't tell the difference between that and local variable declaration).

So, with many apologies for messing around with your code in such a pedantic way, I do recommend that you try and do it the rubyish way, because it'll be kind of necessary if you're going to contribute to other big ruby open source codebases, and it'll also be somewhat expected by others contributing to this one.

The second commit is this change is more of a real refactoring. I added the aasm (acts as state machine) gem and used it in the motion model. This didn't necessarily reduce the number of lines of code, but I could see that you were starting to need to do things based on conditions of both the current value of "phase", and the previous value. This seems like a perfect time to use a state machine. Even with only two states I think it's a little clearer, and I get the impression you're going to add more anyway. Note that the AASM config adds a bunch of magic methods, like closed? voting? open_voting open_voting! and so on. It also adds named scopes, so you get a lot of bang for your buck.