Code review: Learning from others

Introduction
DueDates version 1.1 gets reviewed by the other development teams in class. Team Purple was assigned to review Team Blue's DueDates and assess their version 1.1 code and documentation. The class, also was assigned to evaluate Google Project Hosting's code review tool because of issues from the previous Code Review.

Team Blue's code
Arthur and Erin did some nice things with their code like the user interface that I mentioned in the review of version 1.1. Another method I liked was their Comparator method, which handles the data structure that is used to store the book's due date and information, it also does sorting of values. I modification I thought needed to be done was to handle the due date as a Date type rather than a String. Because strings sorted lexicographically using the character's Unicode which could cause a problem when sorting 11/11/2008 and 4/4/2008 the sort will place 11/11/2008 ahead 4/4/2008.

/**
 * Returns a Comparator object that allows BorrowedItem objects to be compared according to their
 * due dates. The comparator is defined as an anonymous nested class.
 *
 * @return Comparator A comparator to compare BorrowedItem objects according to their due dates.
 */
public static final Comparator dueDateComparator = new Comparator() {
  public int compare(BorrowedItem item1, BorrowedItem item2) {
    int comp = (item1.yyyymmddDueDate).compareTo(item2.yyyymmddDueDate);
    if (comp != 0) {
      return comp;
    }
    // Compare the rest of the fields so that two BorrowedItem objects compare to zero
    // if and only if all their fields are equal.
    String item1Info = item1.libraryName.concat(item1.formattedResults);
    String item2Info = item2.libraryName.concat(item2.formattedResults);
    comp = item1Info.compareTo(item2Info);
    return comp;
  }
};

One area I thought Team Blue could use improvement was with their unit tests. A programmer needs to know how a system will respond to invalid values such as null or inputting an integer when a string was expected. A thorough test will catch most errors and bugs before a user gets to wreak havoc on an application. I suggested to Team Blue to test their Comparator to make sure it sorts what they expect to sort. Another test that should be performed is to make sure exceptions are thrown at the right time. Also, kudos to Team Blue for their homepage, job well done.

Team Purple's review
Team Violet was assigned to review our system and honestly I expected to get blasted in the review, I am always expecting the worst with reviews, but Anthony and Vincent were not too critical. Vincent did point out the expandability issue which does concern me a great deal. Adding more libraries to DueDates will make Team Purple's version one huge class. However, I thought Anthony's assessment of our code was insightful and will assist us in developing a more robust system. The review of Team Purple's system showed deficiencies in our code that will problematic when performing a build. Both Vincent and Anthony had problems with verifying our DueDates due to a JUnit error. Vincent could not successfully import Team Purple's code into Eclipse because of problems with HttpUnit, I think its a build path issue.

Emma coverage tool:
[concat] Emma Coverage summary
[concat] class:   67%  (2/3)
[concat] method:  44%  (15/34)
[concat] block:   18%  (156/874)
[concat] line:    18%  (36/199)

The Emma coverage does not concern me because version 1.1 does require more testing which should improve the coverage. Overall, Team Violet gave us valuable information that helps us improve the system.

Google Project Hosting
In the review of DueDate version 1.0, some flaws of Google's Code Review tool were discovered during the assessment of other DueDates implementations. Google had made some modifications to the Code Reviewer but I think it needs more improvement because it is difficult to locate a review if the reviewer does not provide a link. Although, someone doing an assessment should notify the development team tof a pending review, sometimes the person may forget to send a notice or assume the analysis would be easily found. Perhaps a function, where upon publishing a review a notice is sent to the team.

Conclusion
I am hoping that my review of Team Blue's system was insightful so they can make improvements to their code. Unless I hear differently from the development teams I review I am not sure of what kind of modifications can be done with how I look at a system. But I suppose one area I can improve upon is with explaining how a team should modify their program. I feel that I am not giving a complete picture of what I am trying to convey.

DueDates: Suffering and more coding

Introduction
We continue our work with DueDates and making the system fully operational. For this evolution of development the group needed to implement changes from the peer review, add a sorting mechanism, include the Hawaii State Library, and test all implementations.


Team Purple
As a group the team went through the same process as in version 1.o we felt meeting every weekday for at least a hour was most effective. Meeting on weekends is difficult for the team because of family and work issues. At every meeting the team would talk about what needed to be done with the project. Usually John would think up something new or find a way to modify the code. We did our best to assign the work as evenly as possible. Although some tasks with DueDates 1.1 were more difficult than others. For version 1.1 John wanted me to take the lead role however, due to the complexity of the project and time constraints I had to hand the reins back to John. The approach in which Team Purple is using in my opinion works quite well. My feeling is the more time developers spending working together more the system improves.

1.0 Versus 1.1
Starting a project from scratch is much easier because you do not have that excess baggage of code that needs attending. After the code review and performing some modifications of our own. The project became much more difficult to adjust because we still had to fix or modify the code and still maintain the integrity of the previous version. I found John and myself making constant revisions to the unit tests after discovering modifications to DueDates.java was not working the way it was intended.

The Code
We were able to meet all of the specifications for this weeks assignments however, I think the system is not as functional compared to other teams. DueDates does not have an user interface which is due to time constraints we wanted to make sure version 1.1 met all of the specifications before making additions.

The Issues
One of the biggest hurdles in DueDates was the sort option, when the user invoked -sort[due date or library] the library books should sort in the order of the due date or library. Initially I was going to implement some kind of sorting algorithm. However, Professor Johnson suggested using the Collections.sort method built into the Java library. I went with a Comparable interface that takes in the date, book information, and library code. But the issue with using the Comparable interface is overriding the equals() and hashCode() methods. I have never done an override of the equals or hashCode methods and it took a while to figure out what I needed to do and also John lent his expertise in the matter.

@Override
  public boolean equals(Object obj) {
    
    if (obj instanceof BookInfoRecord) {
      BookInfoRecord pair = (BookInfoRecord) obj;
      return pair.getDueDate().equals(this.dateDue) && pair.getBookTitle().equals(this.bookTitle);
    }
    return false;
  }
  
  /**
   * Overriding default hashCode. 
   * 
   * @return Returning a unique BookInfoRecord hashCode.
   */
  @Override
  public int hashCode() {
    return this.dateDue.hashCode() + this.bookTitle.hashCode();
  }

Continuous Integration
For this week's version of DueDates we were introduced to Continuous Integration using Hudson. The teams got to park their project on Professor Johnson's server. If you have never used Hudson or Continuous Integration, it is like a pet cat or dog if you do something it does not like it will let you know, which can be a letdown. The development team has to always remember to verify before committing and building. Not verifying before a commit should be considered a mortal sin because you hate to see a red dot and rain clouds, which means your current build failed.
Keeping continuous watch on a project's condition is good and bad. The good thing about Continuous Integration is that everyone involved with the system is always aware of its current condition. The team knows the condition of the build, who caused the failure of the build, and why it failed. The bad thing about Continuous Integration, in my opinion is that unnecessary need to keep the sun up. A developer can become obsessed with striving for a blue dot and the sun that they lose focus on the project.

Improvements/Conclusion
To improve my effectiveness, I have to take on more responsibility in the development. With version 1.1 more of my code made it into the distributed version. However, I still would like more lines code making into the next distribution and the only way to that is to continue programming.
I feel with each week I am improving as a programmer I am starting to see it in my code and problem solving. But I still have a long road in front of me I am no where near the likes of John or Ronn.

Code review: The Good, The Bad, The Ugly

Introduction
First off I needed to come up with clever title for the blog and I thought The Good, the Bad, and the Ugly was appropriate for reviewing code. I would like to mention that in no way is the title a reflection of my fellow classmates' programs. I will however, point out what I thought was specifically good, what was generally bad, and make a general statement about what was ugly.

When it comes to programming I am still in that in-between phase of, I know enough code to write a simple application and but do not have enough experience to tackle a full project. Because of my experience, for the review, I could only comment on portions of code I was familiar with and used minimal examples to show what I thought was proper or made more sense in my estimation.

The Good
Overall, the code I saw had good ideas the kind of stuff I wish we implemented in our DueDates. The Blue Team: Arthur and Erin's program did some cool things I especially like their interface which prompts the user to choose a library.



The Green Team: Jeho and Yasu concatenated the values from the respective website which will help to present the information in a meaningful way.

String bookInfo = "";
bookInfo = bookInfo.concat(String.format(format, "Due Dates", "Book Information" ));
for (int i = 1; i < bookinfo =" bookInfo.concat(String.format(format,table.getCellAsText(i,">

The Orange team: Aric and Daniel used an Abstract class and method which I thought was interesting and something I was not thinking of when we were going through the preliminaries of the first DueDates assignment. Using Abstract should be able to give the group the ability to reuse the code when needing to add more libraries to the application.

The Bad
I did not see code that was troubling however, I saw bad documentation and a lack of testing. One group had only one test case and a what looked like a test in the constructor of the unit test. Another group had zero test cases, other than running the code they could not check for valid or invalid values. I saw a class that had one proper Javadoc for a method.

The Ugly
Again, I did not see bothersome code except for trying to read the code to the left (sorry Aric and Daniel), its an implementation of one of their classes with html tags filling every line. My eyes hurt after looking at the file. The code was not bad it was just hard to read as you can see.




Team Purple's code
For the most part the reviewers: John, Arthur, Vincent, Ronn, Daniel, and Yasu liked our implementation with a few of them making suggestions on how to improve our code. One thing I would like to implement is an interface for an user to enter information instead of having to do it through the command line. The team needs to improve the way DueDates retrieves information from webpage with a table. I agree with Ronn's assessment of abstraction however, that should be no problem when we need to make the adjustment in future versions. We do have a nagging xbeans.jar issue that causes an error with JUnit and the verify which needs to be fixed before we can start the next the phase of the project.

Issues with the review tool
I had problems getting the comment window the code review. I was attempting to open the window by clicking on the line number when I needed to be clicking on the line of code. That bit of brain-lock prevented me from reviewing the code for a day. A couple gripes about Google's code review besides having to click a line of code to enact the comment window. With my comments only one set appeared in one of the group's code, the other teams' reviews were stuck in another page so it was difficult to know if the comments were saved. In case I need to make a change to a review it is complicated to find the comments I made to a file.

My Thoughts
Doing code review is helpful by improving the system that is being reviewed and the reviewer gets to see how other programmers are implementing their programs. By looking at people's code a developer may see a whole another way of doing things such as Team Orange's use of Abstract. So for a future project a developer can take advantage of the knowledge they gained by looking at other programmer's code.

Conclusion
We were told to spend a hour on each team's system which was barely enough time to go through the documentation, verification, and examination of the application. Because DueDates is still in its infancy the project is small enough to inspect and still understand how the application is suppose to work. Other than my small mishap with the comment portion of the code review. The only improvement that I see with reviewing code is spending more time examining programs. Also, getting the developement team and the reviewer together to go over the comments, if possible. Such a meeting maybe helpful for both the reviewer and the programmer.

DueDates

Introduction
Finally we were given an assignment that exposes to the class to software engineering. The task for week nine and for the rest of semester is to develop an open source project. DueDates is an application that helps keep track of borrowed books and should evolve into a rental tracker. In this week's blog installment I will talk about the overall development, the team, the issues I encountered, and my opinion of the assignment.







Screen shot of Team Purple's project site.

The distribution, executable, and Javadoc
DueDates source code
DueDates jar file
DueDates Java Docs

The Team
For week nine's development evolution I am assigned to Team Purple which is made up of myself, John Ancheta, and Marilee Flestado. John having the most experience out of the group took the lead role, assigning tasks, choosing the development scheme, and overseeing the entire project. The group met Monday through Friday for at least one hour to mull over code and figure out which issues needed attention. Each time the team met we were able to make meaningful additions to the project whether it was through documentation or coding. With three team members we performed trio programming with John being the conductor. I felt that the process was efficient and should not be altered.

The Development
We decided to deal with only the Univeristy of Hawaii at Manoa library website. The feeling was to keep the project simple, fullfill the requirements first and make additions to the project if there was time. The most challenging aspect of the code was dealing with command line input for the unit test and JUnit build file. John and I investigated Ant's and Java's documentation for a solution. After a day of searching John found an answer to the problem. Java's System class can handle standard input allowing the unit test to take in arguments from the command line.

 /** Instance variable, instantiated to take in a command line argument. */
private static final String "variable" = System.getProperties().getProperty("args");

/** Instance variable, instantiated to take in a command line argument. */
private static final String "variable" = System.getProperties().getProperty("args"); 

To deal with the JUnit build file so the command line arguments could be passed in when invoking ant -f, we made use of sysproperty.

<sysproperty key="<arg1>" value="${<arg1>}"/>
<sysproperty key="<arg2>" value="${<arg2>}"/>

Another interesting aspect about DueDates is working with screen scraping and getting data from a website. The backbone of DueDates is the task of retrieving salient information from a library or rental internet site. To handle the parsing of data we are using HttpUnit an open source website-unit tester. HttpUnit is a nifty tool that allows the developer to check on links, tables, frames, etc.
DueDates is written in Java so it will run on any OS supported by the platform. The nice thing about DueDates is it's open source not only is it free but any Java developer can make a contribution to the project. The big thing I realized about working with open source development is the vast amount tools that is available for free.

The Pitfalls
Being an absolute beginner in project development I felt overwhelmed with the entire process. For starters, I forgot that I needed to update the local directory because I proceeded to commit my version of the code to the repository overwriting Marilee's files.

The Updates page is a mess I was continually making updates to the repository without checking the Issues page and not inputting the issue that I was modifying. After making a commit I would swear at myself for not listing the issue in the comment area thus making the issue un-trackable. After several warnings from John about losing points I finally got the hang of working with SVN client and made doubly sure an issue is listed in the commit.



My Thoughts
The highlights of this assignment, we are working on a true project which is open source and putting to use the tools we learned in the first eight weeks of class. The lowlights of the exercise, to be honest I was quite fearful of the project. After looking at the specifications I was not sure with where to start or what needed to be done. Thankfully John was able to take the lead and guide the team through the process. After learning from John's development experience, I feel confident that I can be a more contributing factor in the stage of the project.

Conclusion
I am excited about this project and hopeful that the class can create a fully functional DueDates application. With each week I am learning something new which adds to my programming experience. I also, see that I do not have to be frighten of the next task because I have the tools in front of me to complete the work.

Configuration Management lab

Introduction
On Wednesday October 6, 2008, the class got more practice working with configuration management and Google Projects. The assignment also involved more pair programming as we performed updates and commits to the respective sites.

The lab
The assignment involved more pair programming, because I had wrapped up the first configuration management tasks over the weekend I was partnered with Arthur Shum, who also finished the exercise. The lab had Arthur and I doing similar tasks as the previous configuration mangement assignment. Since our project sites were already set up it was just a matter of adding each other as a member, downloading and modifying the code. After doing a brief scan through Arthur's code the best thing to do was modify the JavaDocs. The point of the exercise was to get acquainted with Google Projects Hosting and Subversion, not hack our partner's stack implementation. Arthur added the .classpath and .project he also modified the verify.build.xml in stack-arakaki project.
Made an interesting discovery with Eclipse while working on the lab. When importing a project into Eclipse, the sub-directory being worked on does not transfer to the application's workspace. So there is no need to change directories when verifying and committing a project.See the screen shots below to get an example of Google Project Hosting in action.


A screen shot of SmartSVN with an update of stack-arakaki.


A snippet of changes to Arthur's TestClearStack.java, notice the JavaDocs.


Arthur's stack project update page after a commit.


A posted update from Arthur's stack discussion group, notice the changes made to the JavaDocs.


A snippet of a posted update from Daniel's stack discussion group, Arthur added .classpath, .project, and modified the verify.build.xml to the project.

My thoughts
Before taking ICS 413 and working with a configuration manager any alteration of source code was either overwritten or saved along with ten other copies of the same file that took up space on the hard drive. If I was programming a project with a partner the code would be exchanged via email and I assume their method of file management was about as good as mine. So having a tool like Google Project Hosting and Subversion is a real treat because I would hate having to deal with a project that has fifty different classes and over ten thousand lines of code. Keeping track of all of those files without configuration managment is dangerous and complicated. Files could get lost, overwritten, or not updated.
The programs I have worked thus far have been small and easy to manage so the need for configuration management was unnecessary. However, as projects become more complicated I appreciate that we have access to Google Project Hosting and Subversion so we can focus on the programming.

Conclusion
No frustrations with this assignment, things went smoothly and without complications. It was a good exercise in the sense we got more practice with updating, modifying, and committing source files using Subversion.

Configuration Management

Introduction
This week's assignment is a continuation of the class' introduction to the tools of software development. To manage the source code throughout the coding phase we are going to use Google Project Hosting which uses Subversion for configuration management. The exercise involved the installation of a SVN client, modification of source code from Professor Johnson's stack project, and creation of our own Google Project page.

Install
Download the latest version of SmartSVN, yes, I have a Mac and enjoy using the machine's UNIX box. Did not experience any problems with retrieving and installing SmartSVN. The installation portion of the assignment gave me the least amount of frustration.

Modify code from Google project
Retrieving and making changes to source code required some effort on my part. SmartSVN, in my opinion is not intuitive, it took a few minutes to figure out how to connect to ICS 413's repository. The area I had difficulty was with setting up the repository profile. I had issues with locating the menu for adding profiles (see the screen shot below), maybe I should have looked at the tutorial first. After a moment of swearing at the computer, I was able to connect and download all the necessary files to make a changes to the stack's source code. Did not find anything meaningful to alter within either the stack implementation or the unit test. The best thing I could come up with was altering the JavaDoc comments in one of the methods. Upon checking for a successful commit, I found the update section of Google Project Hosting a nice feature. I like that everyone can see who did what to the project. That way you can shoot an email to someone for a job well done or lambast them for screwing up the class.


Screen shot of the update page from stack-johnson.


Screen shot of the trunk from stack-johnson.


Screen shot of the changes I made to stack-johnson.


Screen shot of SmartSVN with the profile manager.

Create a new Google project
Setting up a new Google project seemed like an easy task, adjusting the page and adding members to the site. What should have been a twenty to thirty minute task took over an hour. I was trying to commit my stack-arakaki directory into the repository and was repeatedly denied access. SmartSVN, was attempting to open the project site using the http protocol. After poking around SmartSVN, I discovered when attempting a commit the application was using the wrong setting from the repository profile for my Google Project Hosting site. SmartSVN was trying to call up the site using http. After more searching I found the relocate command and was able to change the protocol to https. It was frustrating to spend so much time on such a simple task.

My Thoughts
Configuration management is not a new topic for me, in ICS 212 (program structure), Ravi Narayan introduced the class to source code management on UNIX using SCCS or Source Code Configuration System. Using SCCS on UNIX is not fun especially when you are bad at keeping track of which version of the source code x.x was updated the previous night. Or a programmer may decide to go with a previous version but has no idea which file holds the code that is needed, which will lead to ineffieciency. So having a GUI configuration manager like SmartSVN is sweet, no need to guess where a paticular version of code is ever again.
With regards to figuring out the issues with protocol and profile I just poked around SmartSVN until I found a solution. Did not feel the need to jump on Google and do a search. Despite being frustrated with portions of the assignment I still find this exercise as well as other tasks thus far, to be valuable. I think it is important to have your patience tested every once in a while for two reasons 1) to keep you humble and 2) to appreciate the small victories when learning something new.

Conclusion
Configuration management through SVN is a necessary tool in software development even when the coding team consists of one programmer. The need to keep track of the constant updates made to a project without the by-products of locking or clobbering is crucial to developers. As will save time and headache allowing the programmers to spend more time in developing code instead of managing code.