It is hard to overestimate the role good code reviews can play in ensuring high quality of any software. Unfortunately, I feel that in many teams reviews mostly become a part of a bureaucratic process, consuming lots of time and resources, but providing very little real added value.
Wouldn’t it be nice to work only on greenfield projects with the team that shares the same vision and style? Unfortunately, we have to spend a significant portion of our professional life dealing with the messy legacy code. Such code is very hard to comprehend. It often consists of tangled classes, with some arbitrary division of responsibilities. Unit test coverage is often very low. Sometimes unit tests are formally there, but you can clearly see that they have been written after the fact, simply repeating the production code mess. These tests are very fragile and don’t provide adequate “safety net”.
Nowadays most software projects set the master branch to be read-only, use pull requests to merge feature branches and require code reviews with multiple approvals. Undoubtedly this a good practice in theory, however, as usual, the devil is in the detail, and quite often typical code reviews are useless and even harmful.
- Most standard code review tools display all changes in a flat list, so the context of these changes is completely lost. The order, in which files appear has nothing to do with the logical organization of the change (and the codebase in general).
- It is extremely hard to reason about changes larger than just a few files with more than just 10-20 added/modified lines of code in each - especially when these changes mix design concepts and low-level implementations of particular functions. It requires a very deep context switch and extreme discipline from the reviewer.
- As a result, many reviews focus only on very simple things. Things that could and should be discovered by code analysis tools, IDE inspections, SonarQube and other automatic quality gates. Reviewer's time is expensive, code analysis tools are cheap.
- When PR is submitted for the review, the code is already written. It is often too late to comment on the fundamental problems in the design. Such comments are hard to address and naturally, they are met with lots of resistance and even anger from the code author.
- Often there is a tight deadline to deliver the feature, so the pressure goes on the reviewer - "It is his fault we didn't deliver the fix on time, he didn't timely approve my code change".
- Typically author of the PR selects the reviewers. This leads to uneven distribution of the reviews (which consume significant time) or sometimes it promotes selecting reviewers that "go easy on everyone".
What’s the solution?
Today’s very short post is about ways to identify technical debt and how to find components/classes that require immediate attention.
I recently read a very interesting article called “Test Desiderata” published by Kent Beck. In this article, Kent describes the properties of good tests. He also mentions that tests of different levels (unit, integration, acceptance, etc.) should focus on different properties.
Validate correctness of the system under test
Document usage and expectations of the tested module
Help designing component’s API and interactions (when practicing TDD)
Provide a safety net that enables fearless refactoring
Two very useful Gradle plugins:
1. axion-release-plugin https://github.com/allegro/axion-release-plugin
There are some very useful command line options you can specify when executing maven tests:
If your server is running in one timezone, but you want to have log messages be printed using different timezone, here is a simple solution:
- Add log4j extras to project classpath (maven GAV is log4j:apache-log4j-extras:1.0)
- In log4j.xml use EnhancedPatternLayout, specifying required timezone for logging purposes (see example below for EST TZ)
In several projects we participated, there was a need to call external Web Services using Java classes generated by wsdl2java Axis tool (or wsdl2java goal of axistools-maven-plugin).
In many projects, especially big ones, developed over several years, there is a lot unused code. Cleaning up projects and deleting such unused code is important.
- Eclipse can detect unused methods and variables.
- PMD (and PMD maven plugin) can also do this and even be integarated into CI process.
- Eclipse Core Tools plugin can find unused classes (see description here).
Unfortunately all above methods will mark code invoked by reflection (e.g. using IoC container such as Spring) as unused. At the same time garbage code that was left behind together with its unit test won’t be detected.
Automatic code format and correcting indentation are essential features of any IDE. Unfortunately FlexBuilder currently doesn’t have them. Recently I found very cool plugin called flexformatter that solves this issue. Start by installing flexformatter using eclipse update site. After restaring Eclipse/FlexBuilder you will find “Flex Formatting” menu under “Preferences”.
I use MacBook as my primary computer at home and Windows in the office.
QuickLook is a feature in Leopard, which allows to view file content without opening it, directly from Finder.
In small applications it makes sense to combine Java and Flex code under same project. I’m a big fan of maven and one of my favorite maven plugins is embedded jetty container. This plugin runs jetty directly from maven and supports automatic monitoring and redeployment of java classes and resources. I thought it would be nice to run Java+Flex projects in the same way and have updated SWF file instantly available in the browser immediately after Flex Builder compiles it.
Note: This is a copy from my old blog posted in 2007. Some things mentioned here have been changed since then.