First look:
I was assigned to review Daniel and Erin's CodeRuler implementation. In the first trial their ruler outperformed against the migrate ruler by a large margin. Unfortunately they weren't too successful against either smart-splitter or gang-up rulers. In both trials (smart and gang) their ruler performed the same way by killing all enemy peasants before capturing its castle. But by the time they captured the enemy's castle, their first castle gets occupied by knights of the opposing team. While they attempt to re-capture their first castle, no peasants are being produced in their second castle. The opposing ruler keeps producing knights after every turn, thus covering and protecting its castle. Their ruler consistantly battles the knights, but unsuccessfully captures back their first castle. For every battle lost between their knights and the opposing team, the enemy gains battle points and they lose precious points from not claiming land. Although the end result is not fully desired, there is much to learn from this implementation.
Review:
Besides what the CodeRuler strategy implies, this review is based on the creators' coding style and class logic structure.
JavaDocs:
The provided JavaDocs for this MyRuler is quite clear and concise. The method summary is descriptive and easy to follow. You can almost get the gist of what each method is intended to do without looking at any code. However, I would like to see a better introduction to their MyRuler class by describing more of their strategy rather than listing the resulting trial matches.
Class logic and structure:
The overall class structure is simple and easy to follow. Each method provides a readable description that matches the code segments. Method references are nicely encapsulated and broken down into smaller reusable routines. I found it much easier to read each function due to the fact that this group followed good naming conventions. As for the effectiveness of their code, I would have to disagree with one routine called: buildUnits (line # 231). I found this method to be less effective in the process of replicating more peasants at each captured castle. This method stops the production of peasants when the team has only one aquired castle. Due to this limitation, other methods relating to the movement of peasants, such as: optimizePeasantMove (line #413) was wasted during gameplay.
Coding style:
Below are some minor code style violations within the MyRuler class. Most of these errors can be set to fix automatically (format) or easily avoided the next time around.
File | Lines | Violations | Comments |
---|---|---|---|
MyRuler | 93-94, | EJS - 5, ICS-SE-Eclipse-2 | opening brace should be at end of the line that introduced the block. |
MyRuler | 87, 88, 175, * | ICS-SE-Eclipse-2 | Some lines exceed the 100 column max limit. |
MyRuler | 155, 159, 177, * | EJS – 7 | Increase white spaces by placing them between keywords (if, for, while, switch) and parenthesis or braces to better readability of code. |
MyRuler | 4 | ICS-SE-Java-2 | Must explicitly specify each class import rather than using a wildcard “*”. |
MyRuler | 130, 133, 146, * | ICS-SE-Java-10 | Needs to pluralize names of collection objects. |
MyRuler | 96, 117, 168, * | EJS - 49 | Omit subject in method description to reduce redundancy. |
Final thoughts:
After reviewing the coding style and class structure of this CodeRuler implementation, I learned that documenting your code is just as hard as implementing it. Maybe a better approach to programming would be to self document first and then extract code statements and method functions from that as you build the application. The all important methodologies of "Top-Down" programming would be an essential skill to practice for even the simplest of classes or methods. This should be applied also to the way we program and document our work. Overall, Daniel and Erin did a good job and I hope they take away as much from this as I did.
Here's a link back to each member's blog. There you can also find a link to their source code.
No comments:
Post a Comment