Thoughts on code reviews (again)

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.

Read More

Static content and webjars in Micronaut applications

Sometimes I need to add a very simple UI to my Micronaut applications. Lightweight javascript libraries such as JQuery or AngularJS are quite convenient for this purpose. Traditionally CDNs are used to reference and download these libraries at runtime, but I prefer to use webjars. This way my app will continue to work even if there is no internet connection or if certain CDN is blocked e.g. by corporate network policies.

Read More

Integration testing of a legacy code

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”.

Read More

Toughts on code reviews and dev process

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?

Read More

Properties of good tests

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.

Read More

Don't over spec your tests

Yesterday I had a discussion with my colleagues about the properties of good tests. I think in general tests have 4 purposes in the following increasing order of importance:
  1. Validate correctness of the system under test
  2. Document usage and expectations of the tested module
  3. Help designing component’s API and interactions (when practicing TDD)
  4. Provide a safety net that enables fearless refactoring
The last point is the most important in my opinion. To provide such safety net, tests must be, as stated by Kent Beck, “sensitive to changes in system behavior, but insensitive to changes in code structure”.
How to achieve this?
Perhaps we should prefer higher-level component/module tests. Such tests are quite more stable and insensitive to structural changes. We should limit the usage of mocks in such tests, probably only mocking collaborators that live outside of the component boundaries.
We should only verify interactions with collaborators that are essential to the business logic of our component.
What do I mean by that? Often I see unit tests where developers stub responses of component collaborators and then verify ALL these interactions. With Mockito they sometimes utilize “lenient” matchers like any() or isA() while stubbing; and “strict” matches like eq() while verifying. This technique is OK, but in my opinion, it should only be applied to true mocks - calls that are essential to the behavior of the system.
Calls to simple data providers (stubs) shouldn’t be verified at all, otherwise, it delivers wrong intentions of the code author as well as makes tests quite fragile.
The difference between stubs and mocks is greatly explained in this article by Martin Fowler.
What do you think? How do you make your tests insensitive to structural changes?
Read More

Useful aliases and ENVs in .profile

alias cp='cp -i'
alias mv='mv -i'
alias df='df -h'
alias du='du -h'
alias grep='grep --color'
alias itest='mvn clean test verify -U'
alias ls='ls -h --color'
alias mci='mvn clean install -U'
alias mi='mvn install'
alias mjr='mvn jetty:run -o'
alias mjrwithprofile='mvn clean jetty:run -DAPP_ENV=dev -Dspring.profiles.active=dev'
alias ps='ps -W -a -f ux'
alias rm='rm -i'
Read More

Control Log4j message timezone

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:

  1. Add log4j extras to project classpath (maven GAV is log4j:apache-log4j-extras:1.0)
  2. In log4j.xml use EnhancedPatternLayout, specifying required timezone for logging purposes (see example below for EST TZ)
<appender name="CONSOLE" class="org.apache.log4j.ConsoleAppender">
    <param name="Threshold" value="TRACE" />
    <layout class="org.apache.log4j.EnhancedPatternLayout">
        <param name="ConversionPattern" value="%d{ISO8601}{EST} %-5p [%t][%c:%M(%L)] %m%n" />
    </layout>
</appender>
Read More

Finding unused code in Java projects

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.

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.

Read More

FlexBuilder code formatter

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”.

Read More

Combined Java+Flex project with hot redeployment

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.

Read More