What is wrong with this code?

It is code review time, and I offer a sample of Java code for you to critique.   Can you find the problems?

Eliminate Useless Comments

Be careful what you ask for.   I was reviewing some code that had absolutely no comments. I asked the developer to add some comment.   So, indeed they did, however the comments added no value to the code at all.   There is no point in adding comments for comments’ sake.  In fact, useless comments make the code worse by adding lines without adding value.   Let’s talk about what a valuable comment is.

Advertisement

Let Me Save Incomplete Files

This is a usability problem that I have run into a number of times.   A programmer writes a UI screen to receive a long list of inputs in order to create an object in system.  Save is enabled only when all the inputs are filled in and verified valid.  This is a big problem.

Return Null or Exception?

I am writing a getter function, and the requested object can not be found.  Do I return null?   Or do I throw an exception?  The answer is “it depends.”  Sometimes both options are needed, but how to decide?

Variable Names & Key Values

Constructing JSON in Java can be a bother.  The most standard JSON library requires objects to be created, and then called to put the members on there.  So much easier in JavaScript, but it is not Java script.  Still, one technique to improve the readability is to name the variable in java to match the key in JSON.   Simple concept, easy to do, I sometimes wonder why it is not obvious to all.

Keep Things the “Same”

This general design principle does not mean to never change anything, but to consciously create a paradigm where the same things have the same names and are accessed in the same way as much as possible.

Constants and Errors

Still reviewing that code base and finding more bad patterns for use of constants, this time for error messages.   This is another “don’t do this” post.

Constant Abuse 2

Some programmers believe that constants are the source of all goodness, because it means in the future that everything will be malliable.  Particularly programmers paid by the hour.  This is a mistake.  Readibility suffers if constants are abused.  I am reading some code today which exemplifies this.

The Urge to Merge

Concurrent development of software in a team relies on being able to merge the changes from each person successfully.  Experienced developers will already know this, but there are theoretical limits on how effective merging can be.

Abstraction and Encapsulation

Abstraction and encapsulation are opposite sides of the same coin, and essential to good object oriented software design.  Why the, is there a tendency of some programmers to go in exactly the opposite direction?  This trend and its implication is discussed below.