Code reviews in the 21st Century

There's an old adage that goes something like: 'Do not talk about religion or politics'.  Why?  Because these subjects are full of strong opinions but are thin on objective answers. One person's certainty is another person's skepticism; someone else's common sense just appears as an a prior bias to those who see matters differently. Sadly, conversing these controversial subjects can generate more heat than light.

All too often people can get so wound up that they forget that the outcome of their "discussion" has no bearing on their life expectancy, their salary, their chances to win x- factor, getting that dream date, winning the lottery, finding a cure for climate change or whatever it is they regard as important!

Similarly, in the world of software engineering code reviews can end up in pointless engagements of conflict.  Developers can bicker over silly little things, offend each other and occasionally catch a bug that probably would have being caught in QA anyway - that conflict free zone around the corner!

Now don't get me wrong, there are perfectly valid reasons why you may think code reviews are a good idea for your project:

1. Catching bugs sooner means less cost to your project. You don't have to release a fix patch because it's has been caught in development phase - yippee!
2. Code becomes more maintable.  That crazy 200 line method that Jonny was writing with a hangover has being caughted before it has the chance to make itself at home deep in your code base.
3. Knowledge is spread across your team. There are no longer large blocks of code that only one person knows about.  And we all know, when that one person talks about taking a two month holiday everyone panics!
4. Developers make more of an effort. If a developer knows someone else is going to pass judgement on his work, he's more likely to put that line of javadoc to clarify when an exception will be thrown.

However, it would be naive to think that code reviews don't cause problems.  In fact, they cause so many problems many 21st century projects don't do them.  I think they have a place but there needs to be some thought regarding how and when they are done so that they are beneficial as opposed to a nuisance.   Here are some guidlines...

• 1. Never forget TDD 

Ensure you have tested your code before you asked someone else to look at it.  Catch your own bugs and deal with them before someone else does.

• 2. Automate as much you can 

There are several very good tools for Java such as PMD, Checkstyle, Findbugs etc  What is the point  getting a human to spend time reviewing code when these tools can very quickly identify many things the human would waste time moaning about?  I am going to say that again.   What is the point  getting a human to spend time reviewing code when these tools can very quickly identify many things the human would waste time moaning about? 

When using these tools, it's important to use a common set of rules for them. This ensures your code is at some sort of agreed standard and much of what used to happend in an old fashioned 20th century code review, won't need to happen.  Ideally, these tools should be run on every check in of code by a hook from your version control system.  If code is really bad - it will be prevented from being checked in.  Billy the developer is prevented from checking in the rubbish he wrote (when he had killer migraine) that he is too embarrassed to look at.  You are actually doing him favours, not just your team. 

• 3. Respect Design 

In some of the earlier Java projects I worked on, the reviews happened way too late.  You were reviewing code when the actual design was flawed.  A design pattern was misunderstood, some nasty dependencies were introduced or a developer just went way off on a tangent.  The review would bring up these points.  The proverbial retort was: 'This is a code review not a design a review!' .  A mess inevitably ensued.  To stop these problems we changed things so that anyone asked to review code would also be involved - in some way - in either the design or the design review.  In fact, we got much more bang from the buck from design reviews than code reviews.  Designs were of a much higher quality and those late nasty surprises stopped.

• 4. Agree a style guide (and a dictionary) 

Even with the automated tooling (such as Checkstyle, Findbugs etc), to avoid unnecessary conflict on style, your project should have a style guide. Stick to the industry standard java conventions - where possible.  Try to have a 'dictionary' for all the concepts your project introduces. This means, when code refers to them it's easier to check that the usage and context is correct.

• 5. Get the right tooling

If all your developers are using Eclipse (and happy using it) something like Jupiter makes sense.  You can navigate your way through code, debug code and essentially leverage everything the Eclipse IDE does to make your life looking at code easier when reviewing code.  However, if everyone is not on the same IDE (or the IDE is not making your life easier) consider something like Review Board. 

• 6. Remember every Project is different.

You may have done something in a previous project that worked.  But remember, every project is different.  The other project had a certain architecture (may have been highly concurrent, highly distributed), had a certain culture (everyone may have enjoyed using eclipse) and used certain tools (maven or ant).  Does the new one tick the same boxes?  Remember, different things work for different projects.

• 7. Remember give and take

When reviewing be positive, be meticulous but do not be pedantic.  Will tiny trivial things that get on your nerves make a project fail or cost your company money?  Probably not.  Put things in perspective.  Remember to be open to other ideas and to change your own mind rather than getting hung up changing someone else's.

• 8. Be buddies 

In my experience, what I call 'buddy reviews' (others call 'over the shoulder')  have worked really well.  A buddy review consists of meeting up with another team member informally every day or two and having a quick glance (5 - 10 mins)  at each other's code at your desk or their's.  This approach means:

1. Problems are caught very early
2. You are always up to speed as to what is going on
3. Reviews are always very short because you are only looking at new code since the last catch up
4. Because the setting is informal - there is no nervous tension. They're fun!
5. You can exchange ideas - regularly. 

When Tech Leading, buddy reviewing your team members is a great way of seeing if anyone on the team is running into trouble early rather than late.  You can help people and get an idea of everyone's progress all at the same time.  And because of the regular nature of buddy reviews, they become habitual and actually get done.  Something we can't say for many other 21st century code reviews!

In summary, if your project is going to engage in code reviews, they should be fast, effective and not waste people's time.  As argued in this post, it is really important to think about how they are organised to ensure that does not happen.

'Til the next time - take care of yourselves.

Reference: Code reviews in the 21st Century from our JCG partner Alex Staveley at the Dublin's Tech Blog .

Read more: http://www.javacodegeeks.com/2012/02/code-reviews-in-21st-century.html#ixzz1lTuBthD1

Database Design Best Practices

1. Use well defined and consistent names for tables and columns (e.g. School, StudentCourse, CourseID ...).
2. Use singular for table names (i.e. use StudentCourse instead of StudentCourses). Table represents a collection of entities, there is no need for plural names.
3. Don’t use spaces for table names. Otherwise you will have to use ‘{‘, ‘[‘, ‘“’ etc. characters to define tables (i.e. for accesing table Student Course you'll write “Student Course”. StudentCourse is much better).
   
4. Don’t use unnecessary prefixes or suffixes for table names (i.e. use School instead of TblSchool, SchoolTable etc.).
5. Keep passwords as encrypted for security. Decrypt them in application when required.
6. Use integer id fields for all tables. If id is not required for the time being, it may be required in the future (for association tables, indexing ...).
7. Choose columns with the integer data type (or its variants) for indexing. varchar column indexing will cause performance problems.
8. Use bit fields for boolean values. Using integer or varchar is unnecessarily storage consuming. Also start those column names with “Is”.
9. Provide authentication for database access. Don’t give admin role to each user.
10. Avoid “select *” queries until it is really needed. Use "select [required_columns_list]" for better performance.
11. Use an ORM (object relational mapping) framework (i.e. hibernate, iBatis ...) if application code is big enough. Performance issues of ORM frameworks can be handled by detailed configuration parameters.
12. Partition big and unused/rarely used tables/table parts to different physical storages for better query performance.
13. For big, sensitive and mission critic database systems, use disaster recovery and security services like failover clustering, auto backups, replication etc.
14. Use constraints (foreign key, check, not null ...) for data integrity. Don’t give whole control to application code.
15. Lack of database documentation is evil. Document your database design with ER schemas and instructions. Also write comment lines for your triggers, stored procedures and other scripts.
16. Use indexes for frequently used queries on big tables. Analyser tools can be used to determine where indexes will be defined. For queries retrieving a range of rows, clustered indexes are usually better. For point queries, non-clustered indexes are usually better.
17. Database server and the web server must be placed in different machines. This will provide more security (attackers can’t access data directly) and server CPU and memory performance will be better because of reduced request number and process usage.
18. Image and blob data columns must not be defined in frequently queried tables because of performance issues. These data must be placed in separate tables and their pointer can be used in queried tables.
19. Normalization must be used as required, to optimize the performance. Under-normalization will cause excessive repetition of data, over-normalization will cause excessive joins across too many tables. Both of them will get worse performance.
20. Spend time for database modeling and design as much as required. Otherwise saved(!) design time will cause (saved(!) design time) * 10/100/1000 maintenance and re-design time.

Read more: http://www.javacodegeeks.com/2012/02/20-database-design-best-practices.html#ixzz1lTpp7AmH