Summary of some common problems in Java programming

  • 2020-04-01 03:30:35
  • OfStack

This article lists some of the more typical errors I've seen in my colleagues' Java code. Obviously, static code analysis (our team USES qulice) is not going to find all the problems, which is why I'm listing them here.

If you feel that something is missing, please feel free to comment and I will be happy to add it.

All of the errors listed below are basically related to object-oriented programming, especially OOP in Java.

The name of the class

Read this passage under "(link: https://github.com/yegor256/d29/wiki/What-is-an-Object%3F). Classes should be real life abstract entities, not "validators", "controllers", "managers", etc. If your class name ends in "er" -- that's a bad design.

Of course, utility classes are also anti-patterns, such as Apache's StringUtils, FileUtils, and IOUtils. These are all examples of bad design. Read on: (link: http://www.javacodegeeks.com/2014/05/05/oop-alternative-to-utility-classes.html).

Of course, do not use prefixes or suffixes to distinguish between classes and interfaces. For example, these names are wrong: IRecord, IfaceEmployee, or RecordInterface. In general, the interface name should be the name of a real-life entity, and the class name should describe its implementation details. If the implementation is not particularly illustrative, you can call it Default, Simple, or something similar. Such as:


class SimpleUser implements User {};
class DefaultRecord implements Record {};
class Suffixed implements Name {};
class Validated implements Content {};

The method name

Methods can return either a value or a void. If a method returns a value, its name should say what it returns, for example (never use the get prefix) :


boolean isValid(String name);
String content();
int ageOf(File file);

If it returns void, then its name should say what it does. Such as:


void save(File file);
void process(Work work);
void append(File file, String line);

There is only one exception to these rules -- JUnit's test method doesn't count. We're going to talk about that.

The name of the test method

In JUnit's test case, the method name should be an English statement with no Spaces. An example will make it clearer:


/**
 * HttpRequest can return its content in Unicode.
 * @throws Exception If test fails
 */
public void returnsItsContentInUnicode() throws Exception {
}

The first sentence in your JavaDoc should begin with the name of the class you are testing, followed by a can. Therefore, your first sentence should be something like "somebody can do something".

The method name is the same, but there is no topic. If I put a topic in the middle of the method name, I get a complete sentence, as in the example above: "HttpRequest returns its content in unicode."

Note that the name of the test method does not begin with can. Only comments in JavaDoc begin with can. In addition, method names should not begin with a verb.

In practice, it is best to declare the test method to throw an Exception.

The variable name

Avoid combining variable names, such as timeOfDay, firstItem, or httpRequest. This is true for class variables as well as for variables within methods. The variable name should be long enough to avoid ambiguity in its visible scope, but not too long if possible. The name should be a singular or plural noun, or an appropriate abbreviation. Such as:


List<String> names;
void sendThroughProxy(File file, Protocol proto);
private File content;
public HttpRequest request;

Sometimes, if the constructor wants to save its arguments into a newly initialized object, its arguments and the names of the class properties may conflict. In this case, I suggest dropping the vowels and using abbreviations.

Example:


public class Message {
  private String recipient;
  public Message(String rcpt) {
    this.recipient = rcpt;
  }
}

Many times, just looking at the class name of a variable will tell you what to call it. I'll just use it in lowercase, it works like this:


File file;
User user;
Branch branch;

However, never do this with basic types such as Integer number or String String.

If there are multiple variables with different properties, consider using adjectives. Such as:


String contact(String left, String right);

A constructor

Exceptions aside, there should be only one constructor for storing data in an object variable. Other constructors call this constructor with different arguments. Such as:


public class Server {
  private String address;
  public Server(String uri) {
    this.address = uri;
  }
  public Server(URI uri) {
    this(uri.toString());
  }
}

One-off variable

One-time variables should be avoided at all costs. By "once," I mean variables that are used only once. Here's one:


String name = "data.txt";
return new File(name);

The above variables are used only once, so the code can be rewritten as follows:

return new File("data.txt");

Sometimes, in rare cases -- mostly for better formatting -- one-off variables may be used. However, it should be avoided as much as possible.

abnormal

Needless to say, never swallow an anomaly by yourself, but pass it as far up as you can. Private methods should always throw checked exceptions out.

Do not use exceptions for flow control. For example, the following code is wrong:


int size;
try {
  size = this.fileSize();
} catch (IOException ex) {
  size = 0;
}

What if IOException says "disk full"? Would you still think that this file size is zero and go ahead and do it?

The indentation

The main rule for indentation is that the left parenthesis either closes at the end of the line or on the same line (as opposed to the right parenthesis). For example, the following is incorrect because the first left parenthesis is not closed on the same line, and there are other characters after it. The second parenthesis is also problematic because it is preceded by a character, but the corresponding open parenthesis is not on the same line:


final File file = new File(directory,
  "file.txt");

The correct indentation should look like this:

StringUtils.join(
  Arrays.asList(
    "first line",
    "second line",
    StringUtils.join(
      Arrays.asList("a", "b")
    )
  ),
  "separator"
);

The second important rule about indentation is to write as many characters as possible on the same line -- up to 80 characters. The above example does not satisfy this, it can also be contracted:

StringUtils.join(
  Arrays.asList(
    "first line", "second line",
    StringUtils.join(Arrays.asList("a", "b"))
  ),
  "separator"
);

Redundant constant

When you want to share information in a class's methods, you should use class constants, which should be specific to your class. Don't use constants as substitutes for strings or numeric literals -- this is a very bad practice and can pollute your code. Constants (like any object in OOP) should have their own meaning in the real world. See what these constants mean in real life:


class Document {
  private static final String D_LETTER = "D"; // bad practice
  private static final String EXTENSION = ".doc"; // good practice
}

Another common mistake is to use constants in unit tests to avoid redundant strings or literal values in test methods. Don't do it! Each test method should have its own input value.

Use new text or values in each new test method. They are independent of each other. So why do they share the same input constant?

Test data coupling

Here is an example of data coupling in the test method:


User user = new User("Jeff");
// maybe some other code here
MatcherAssert.assertThat(user.name(), Matchers.equalTo("Jeff"));

In the last line, "Jeff" is coupled to the same string literal in the first line. If, after a few months, someone wants to change the value of the third row, he or she will have to spend time figuring out where the same "Jeff" is used in the same method.

To avoid this, you'd better introduce a variable.


Related articles: