ronn.reeves\software.engineering.blog

In Lessons Learned From CodeRuler, I explored how that project influenced my understanding of the Java language, the Eclipse IDE, and especially, peer programming. In this article I continue that exploration of peer programming by examining and evaluating the CodeRuler implementation from a group of peers in my ICS 413 course this semester. The main aspect of the evaluation is the adherence to Java programming standards defined by these sources:

• The Elements of Java Style by Vermeulen, Ambler, Bumgardner, Metz, Misfeldt, Shur, & Thompson• ICS 413 Supplemental Standards from Philip Johnson
The Elements of Java Style (EJS) had been informally introduced prior to the CodeRuler assignment to the class, but had not yet been mandated as the formal coding style to which course work should adhere. The latter standards, referred to as ICS-SE-Java or ICS-SE-Eclipse, were introduced to the class after completion of the CodeRuler project. Doing the CodeRuler assignments in this manner provides each developer within the class the opportunity to discover his/her baseline adherence to the standards expected through code reviews of our peers’ course work. By discovering deviations from the standards by our peers, we learn to look for deviations in our own programming methodology. Additionally, this assignment begins our exposure to the expectations that will be placed on us as programmers entering into the workforce, as well as laying down a foundation of programming best practices and tools we will need for our eventually rise into management and executive positions.

Peers Being Reviewed
For the purpose of this review, I will be examining the CodeRuler implementation provided by

• Arthur Shum (Check out his CodeRuler blog entry.)• Aric West (Check out his CodeRuler blog entry.)
Their implementation may be downloaded directly from either’s blog or from here:

• aricwest-atshum.zip
The Good
To say that this implementation provides an effective solution to the CodeRuler challenge would be an understatement. Even its designers describe it as an “aggressive” solution. Ruthless is probably a better word. This implementation is quite successful, and after watching it successfully conquer ruler after ruler in just a few moments of game play, even in 3 to 6 ruler competitions, I had difficulty finding a better suited word than “ruthless” to describe West-Shum’s implementation. Their ruler does quite a good job of dispatching opposing rulers and their forces in a very short period. The salt-in-the-wound for opposing rulers is West-Shum’s boxing of enemy peasants by using their peasants such that the opposing peasants are unable to move, and this on top of having already decimated all opposing offensive and defensive units.

The supporting JavaDocs are in order with short, concise descriptions of how to use their implementation. The documentation is easy to read, well-organized, and defines the class and implementation features appropriately.

For the most part, the class definition (MyRuler.java) is organized appropriately being arranged such that readability is reinforced.

The Bad
The following table displays the violations of the EJS, ICS-SE-Java, and ICS-SE-Eclipse standards occurring within the West-Shum implementation of CodeRuler.

Source Code File:  MyRuler.java

* Denotes this violation occurs in additional lines of code not indicated.
And What I’d Suggest
By in large these violations of the standards are minor issues that can easily be addressed by these adept programmers. Their ability to program well is evident in the success of their solution, and being as such, addressing these minor infractions will require little more than an awareness of them. However, I have two observations regarding the implementation that I personally would change: 1) keeping the methods smaller and focused to a single task, and 2) more appropriate use of member fields. Regarding the first observation, the orderSubjects method performs 6 tasks within its body: gather information, create knight teams, prioritize enemies, attack enemies, order peasants, and castle production. I would suggest breaking most of these code blocks into separate methods, such that orderSubjects reads similar to

   1  public void orderSubjects(int lastMoveTime) {
   2    //Gather information
   3    this.myPeasants = getPeasants();
   4    this.myKnights = getKnights();
   5    this.myCastles = getCastles();
   6    this.enemyCastles = World.getOtherCastles();
   7    this.enemyKnights = World.getOtherKnights();
   8    this.enemyPeasants = World.getOtherPeasants();
   9        
  10    createTeams();
  11    prioritizeEnemies();
  12    attackEnemies();
  13    orderPeasants();
  14    controlProduction();
  15  }

Of course, this is just my personal style, but I prefer this style due to its readability and the ability to focus on the specific components in the larger method.

The second observation is that some variables seem to be unnecessarily long lived. For example, several field members are created (i.e., myCastles, myKnights, myPeasants), then initialized on each call of the orderSubjects method. These are then passed as method parameters throughout the rest of the implementation, instead of using direct calls to them. There is one instance where the myKnights member field is referenced directly, but it could just as easily been passed as a parameter by the calling method. I don’t see any benefit to making these member fields, since they are not used in that capacity within the implementation. Personally, I would just make these method level variables and pass as method parameters as needed from the orderSubjects method.