ifpingram/tdd-kata-1 — pull request #1 by drsharp (2014-07-09 23:58:33+00:00)
Title: Clean up and normalize code styling

I like that the tests are thorough and easy to follow. I cleaned them up just a teeny bit. I also refactored the Calculator class to conform to more of what I see as typical Ruby styling. Ruby is powerful enough that it worked just fine the the way you had it, but I wanted to suggest some styles/forms that are more in-line with the community.

To wit:

1. You don't need to declare initialize as a method if you aren't doing anything. Every class has that by default. You only declare it if you are taking specific parameters and/or doing any setup.

2. Classes/Modules start with a capital letter or are CamelCase. Methods are lower-case with underscores between words. So I made the method name be "add" instead of "Add".

3. You don't need parentheses, but I often put them in to make the code more readable. So I added some in some places and took some away in other places. You can decide what you like. The general community consensus seems to be: put them where it adds clarity but otherwise leave them off (like if there are no parameters).

4. Ruby has a nice couple of methods for taking an array, altering it, then doing an operation on it's members and returning the collected result: map/reduce. You can see what I did. Also, you can pass an &:something form and it will call the 'something' method on each member of the collection (thus, turning them to integers). 'reduce' is also the same as 'inject'. It's a way to pass an operation on a collection and it will iterate through keeping a temporary holder variable as it does the operation and then returning the result. BTW, I added a default value to reduce() so that it will return 0 instead of nil if the collection is empty. (one test failed without that).

5. Ruby provides 'to_s' for almost everything by default. The only time to (re)define it is if you want to create your own meaning of what an object should look like when represented as a String. However, It's probably best to either leave it alone or still make it "string-like". Returning "true" for to_s seems a little misplaced. I changed that test to just verify that the instance object is a type of Calculator object. That's often good enough to verify that the object created exists and is of the right type.