views:

118

answers:

4

Hello,

I have just created a mid-sized web-application using Java, a custom MVC framework, javascript. My code will be reviewed before it's put in the productions servers (internal use).

The primary objective of building this app was to solve a small problem for internal use and understand the custom made MVC framework used by my employer. So, my app has gone through MANY iterations, feature changes and additions.

So, bottom line, the code is very very dirty and this is my first "product level" Java app.

What are your suggestions, what are some basic checks/refractoring I should do before the code review?

I am thinking about:

  1. Java best practices (conventions)

  2. Make the code simple to understand for the developer who will maintain it. (won't be me)

  3. I noticed, I have created some unnecessary objects and used hashmaps/arraylists where could have easily used some other Data structure and achieved the solution. So, is that worth changing?

Update

Your Code Sucks and I Hate You: The Social Dynamics of Code Reviews

+2  A: 

Unit tests, and they should be automated as part of your build. You should already have these, but if not, do it now. It will definitely make the refactoring easier, as well improving your general confidence in the code (and the guy who will be maintaining it).

Noel M
Is it *REALLY* important? I have 3-4 days left for the code review.
zengr
Yes it is, do this before other changes as they may cause regressions that these will show.
Tom
+1  A: 

Logging.

One of the more overlooked things is the importance of logging. You need to have a decent logging methodology put in place. Even though this is an internal app, make sure that the basic logs can help regular users find issues and provide more detailed logging so that you (the developer) would know where to go.

bogertron
Good point on logging
zengr
+1  A: 

Comment your code, explain why it's doing what it's doing and what assumptions have been made.

Try to reduce the amount of mutating state.

Try to remove any singletons you may have.

Tom
+1  A: 

If you did not already, (assuming you use an IDE like eclipse)

  • get plugins checkstyle and findbugs
  • go through their configuration and tune to your style
  • run them on your code
  • resolve all issues reported

you can also tune the compiler warning setting of eclipse itself and possibly make them more strict in what is reported.

Look at code structure:

  • get plugin jdepend
  • investigate your package structure

Code against interfaces (Map, List, Set) instead of implementation classes (HashMap, ArrayList, TreeSet)

Complete your Javadoc and make check it is up to date after all refactorings.

Add JUnit tests; if you have no time left to test the whole application, at least create a test for every bug you find and solve from now on. This helps "growing" a test set as you go.

Next time design and build your application with the end goal in sight. Always assume that the next guy having to maintain your code will know how to find you :-)

rsp