1. Accessing non-static members from static methods (mostly from the main method)
2. Missing closing curly braces
3. Missing break in switch case construct
4. Confusing assignment with comparison (= and ==)
5. Confusing object comparison (== instead of .equals)
6. Confusing about 0-based or 1-based index
7. Using less restrictive access modifiers
8. Forgetting to free resources
9. Ignoring Exceptions
10. Modifying a collection while iterating it
public class StaticExample1 { public int number; // instance variable public static void main(String[] args) { number = 30; // compile error } }Java compiler will issue this error:
error: non-static variable number cannot be referenced from a static contextHere, number is an instance variable which means that we can only access it via an object of its class, as shown in the following correction:
public class StaticExample1 { public int number; // instance variable public static void main(String[] args) { StaticExample1 obj = new StaticExample1(); obj.number = 30; } }Another solution is making the variable become static, but it depends on the program’s purpose. For example, it’s fine for the following program to access a constant (static final field):
public class StaticExample2 { public static final int BUFFER_SIZE = 4096; // a constant public static void main(String[] args) { byte[] data = new byte[BUFFER_SIZE]; } }The same applies for an instance method being accessed by a static method. For example:
public class StaticExample3 { // instance method: public int sum(int a, int b) { return a + b; } public static void main(String[] args) { int a = 399; int b = 256; int c = sum(a, b); // compile error } }
public class StaticExample3 { // instance method: public int sum(int a, int b) { return a + b; } public static void main(String[] args) { int a = 399; int b = 256; StaticExample3 obj = new StaticExample3(); int c = obj.sum(a, b); } }So to avoid this mistake, remember the following rules:
Thread stopper = new Thread(new Runnable() { public void run() { try { Thread.sleep(RECORD_TIME); } catch (InterruptedException ex) { ex.printStackTrace(); } recorder.finish(); });Yes, you can see that there is a missing closing brace for the method run() although the code looks seemingly fine due to the last two lines are indented improperly. Here’s the correction:
Thread stopper = new Thread(new Runnable() { public void run() { try { Thread.sleep(RECORD_TIME); } catch (InterruptedException ex) { ex.printStackTrace(); } recorder.finish(); } });Although this mistake is detected quickly by the compiler and instantly by modern IDEs, programmers still often make it. So to avoid this mistake:
Scanner scanner = new Scanner(System.in); int option = scanner.nextInt(); switch (option) { case 1: System.out.println("1 -> New"); break; case 2: System.out.println("2 -> Edit"); break; case 3: System.out.println("3 -> Delete"); break; case 4: System.out.println("4 -> View"); break; case 5: System.out.println("5 -> Quit"); default: System.out.println("Unknown command"); }Can you spot the missing break in this code? Let run the code and enter 5 as input and you will get the following output:
5 -> Quit Unknown commandOops! It’s because there is a missing break in the 5th case, so the execution falls through to the default case. Here’s the correction:
case 5: System.out.println("5 -> Quit"); break; default: System.out.println("Unknown command");This mistake is a little bit difficult to detect because it doesn’t cause compile error. We only realize the problem when the program runs not as expected.However, sometimes we do need to remove the breaks, as shown in the following method:
public static int getDaysOfMonth(int month, int year) { switch (month) { case 1: case 3: case 5: case 7: case 8: case 10: case 12: return 31; case 4: case 6: case 9: case 11: return 30; case 2: return (year % 4 == 0) ? 29 : 28; default: throw new IllegalArgumentException(); } }As you can see, this method returns number of days for a given month and year, and the fall-through feature of the switch case is a great help.Read this article to fully understand about the switch case construct in Java.
Scanner scanner = new Scanner(System.in); int number = scanner.nextInt(); if (number = 1) { System.out.println("Choose 1"); } else { System.out.println("Choose other"); }Here, number = 1 is an assignment not a comparison, thus the compiler detects this mistake easily. Correction:
if (number == 1) {But watch out for boolean assignment which the compiler cannot detect, as shown in the following example:
public void openFile(String path, boolean readOnly) { if (readOnly = true) { // this is always true } }The if (readOnly = true) seems to be a comparison but it is an assignment and the variable is of type boolean so the code compiles normally. The problem is, code inside the if block always get executed regardless of the value of the argument. Correction:
if (readOnly == true) {Consult this article to understand more about operators in Java.
public void getMeat(String type) { if (type == "beef") { System.out.print("Choose beef"); } else if (type == "pork") { System.out.print("Choose pork"); } }This seems to work correctly with the following call:
getMeat("pork");This works because Java caches String literals in String constant pool so that the == works here. But this won’t work:
String meatType = new String("pork"); obj.getMeat(meatType);Why? Because meatType is now a different String object so the == operator returns false. The rule is always using the equals() method to compare two objects, unless you have reason to compare two objects directly.Thus here’s the correction for the above method:
public void getMeat(String type) { if (type.equals("beef")) { System.out.print("Choose beef"); } else if (type.equals("pork")) { System.out.print("Choose pork"); } }Also pay attention when comparing two Integer wrapper objects. Consider the following code:
Integer i1 = 100; Integer i2 = 100; System.out.println(i1 == i2);This will print true, but the following code prints false:
Integer i1 = 500; Integer i2 = 500; System.out.println(i1 == i2);Why? It’s because Java caches integer numbers ranging from -128 to 127. So remember the rule of using equals() method to compare objects.Refer to this article to understand more about the equals() method.
String[] fruits = {"Apple", "Banana", "Carrot", "Grape"}; String firstFruit = fruits[1]; // should be: fruits[0]The problem is even worse if the array has only one element, then an ArrayIndexOutOfBoundsException will occur. For example:
int[] numbers = {256}; int firstNumber = numbers[1]; // cause ArrayIndexOutOfBoundsExceptionAnd it’s worth paying attention to some methods whose index parameters are both 0-based and 1-based. Let’s take the method substring(beginIndex, endIndex) of the String class for instance: the beginIndex is 0-based, but the endIndex is 1-based.
String title = "JavaProgramming"; String subTitle = title.substring(0, 4); System.out.println(subTitle);This gives “Java” as the result.Likewise, the method subList()of List collections has fromIndex starts from 0 and toIndex starts from 1. For example:
List<String> names = Arrays.asList("Andy", "Bob", "Carol", "David", "Eric"); List<String> subNames = names.subList(0, 3); System.out.println(subNames);This gives the result: [Andy, Bob, Carol]So you can infer the rule for such methods: begin index starts from 0 and end index starts from 1.Also be aware of the Calendar class which returns date on 1-based but returns month in 0-based. For example:
Calendar calendar = Calendar.getInstance(); // current date is November, 2nd 2016 calendar.setTimeInMillis(System.currentTimeMillis()); int date = calendar.get(Calendar.DATE); int month = calendar.get(Calendar.MONTH); System.out.printf("date = %d, month = %d", date, month);This prints: date = 2, month = 10So bear in mind these rules to avoid this beginner mistake.
public > protected > default > private
These access modifiers are sorted from the least restrictive (public) to the most restrictive (private). However, novice programmers do not fully understand this and often use the public and default access modifiers which make the code less secured.Consider the following class:public class Person { public String name; int age; public Person(String name, int age) { if (name == null || age < 1 || age > 100) { throw new IllegalArgumentException("Invalid name or age"); } this.name = name; this.age = age; } }It seems to be fine as the constructor validates the name and age to make sure they are properly set. For example, this instantiation looks fine:
Person p1 = new Person("Alex", 30);And this instantiation causes an exception to be thrown, as expected by the logic in the constructor:
Person p2 = new Person(null, 0);But the problem is, the member variables name and age are not protected (name can be accessed everywhere and age can be accessed in the same package), one can easily bypass the check logic in the constructor by writing code like this:
Person p1 = new Person("Alex", 30); p1.name = null; // valid p1.age = -1; // validSo to protect member variables from unwanted changes, we need to use more restrictive access modifier such as private (cannot be modified outside the class) or protected (can be modified only by subclasses or classes in the same package). Thus the following is the correction for the Person class:
public class Person { private String name; private int age; public Person(String name, int age) { if (name == null || age < 1 || age > 100) { throw new IllegalArgumentException("Invalid name or age"); } this.name = name; this.age = age; } }So remember this rule: always use the most restrictive access modifiers if possible.Consult this article to fully understand about access modifiers in Java.
public static void copyFile(File sourceFile, File destFile) { FileChannel sourceChannel = null; FileChannel destChannel = null; try { sourceChannel = new FileInputStream(sourceFile).getChannel(); destChannel = new FileOutputStream(destFile).getChannel(); sourceChannel.transferTo(0, sourceChannel.size(), destChannel); } catch (IOException ex) { ex.printStackTrace(); } }As you can see, this code misses the statements to close the sourceChannel and destChannel after using them. A solution to this is using the try-with-resources structure available since Java 7, which automatically closes the resources. For example, the above code can be re-written like the following code:
public static void copyFile(File sourceFile, File destFile) { try ( FileChannel sourceChannel = new FileInputStream(sourceFile).getChannel(); FileChannel destChannel = new FileOutputStream(destFile).getChannel(); ) { sourceChannel.transferTo(0, sourceChannel.size(), destChannel); } catch (IOException ex) { ex.printStackTrace(); } }Therefore, to never forget freeing resources, use the try-with-resources structure on the resources implement the AutoCloseable interface.
public class Sum { public static void main(String[] args) { int a = 0; int b = 0; try { a = Integer.parseInt(args[0]); b = Integer.parseInt(args[1]); } catch (NumberFormatException ex) { } int sum = a + b; System.out.println("Sum = " + sum); } }As you can see, this program calculates the sum of two numbers passed via command-line arguments. Note that the catch block is left empty. If we try to run this program by the following command line:
java Sum 123 456yIt will fail silently:
Sum = 123It’s because the second argument 456y causes a NumberFormatException to be thrown, but there’s no handling code in the catch block so the program continues with incorrect result.So to avoid such potential problem, always handle exceptions at least by printing the stack trace to inform the error when it happens:
try { a = Integer.parseInt(args[0]); b = Integer.parseInt(args[1]); } catch (NumberFormatException ex) { ex.printStackTrace(); }Having said that, don’t be lazy to ignore exceptions, as writing a simple a print stack trace statement takes only few seconds and it can save you hours of debugging later if the problem occurs.
List<String> fixedList = Arrays.asList("Apple", "Banana", "Carrot", "Grape"); List<String> listFruit = new ArrayList<>(fixedList); for (String fruit : listFruit) { if (fruit.contains("e")) { listFruit.remove(fruit); } }This code attempts to remove String elements containing “e” letter but it causes ConcurrentModificationException at runtime, even in a single-threaded program. Why? There are two main reasons that Java prohibits this kind of operation:
So is there any solution?Yes, there’s a few.First, we can build a list contains elements that needs to be removed, then iterate on this list to remove elements in the original list. For example:
List<String> fruitToRemove = new ArrayList<>(); for (String fruit : listFruit) { if (fruit.contains("e")) { fruitToRemove.add(fruit); } } for (String fruit : fruitToRemove) { listFruit.remove(fruit); }Second, a more compact solution is using an iterator which is designed for modifying a collection while iterating its elements. The following example works well:
Iterator<String> iterator = listFruit.iterator(); while (iterator.hasNext()) { String next = iterator.next(); if (next.contains("e")) { iterator.remove(); } }Third, Java 8 makes thing much simpler with the Streams API. For example, the following code produces same result as the above code:
List<String> result = listFruit.stream() .filter(fruit -> !fruit.contains("e")) .collect(Collectors.toList());That’s the 10 common mistakes in Java programming which every novice programmer makes in his or her life. Note that this is not a top 10 because we don’t have reliable statistics data.We hope you find this article helpful and we are welcome for suggestions for other common mistakes in Java.