The “for”, “while” and “do-while” statements repeat application logic. Programmers often embed two pieces of logic in the same loop statement: one repeats N times and the second N-1 times. For example, concatenation of a string array to produce the comma separated output joins N array elements with N-1 commas in between. While doing code reviews I have seen one bug many times during my career so I decided to blog about it. In this post I will show broken and correct code examples of simple re-try logic.

The code below opens a file using simple re-try logic. If the file does not exist the function waits for 100 milliseconds and tries opening the file again. This method has the tryCount parameter to control the number of attempts to open the file. Don’t copy/paste the code as it contains a bug:

   public BufferedReader tryOpenFile1(String fileName, int tryCount) throws IOException {
      Path path = FileSystems.getDefault().getPath(fileName);
      while (tryCount-- > 0) {
         System.out.println("Check if file exists. Try " + tryCount);
         if (Files.exists(path)) {
            return Files.newBufferedReader(path, Charset.forName("UTF-8"));
         }
         sleep(100);
      }
      return null;
   }

   protected void sleep(int milliseconds) {
      try {
         System.out.println(String.format("Sleeping for %s milliseconds", milliseconds));
         Thread.sleep(milliseconds);
      } catch (InterruptedException e) {
         e.printStackTrace();
      }
   }

The tryOpenFile1 method works and commonly written unit-tests pass, but this method contains a bug. The method repeats “sleeping” logic N times when it should do that N-1 times. This bug leads to the extra sleeping time period after the last unsuccessful attempt occurred to open the file. This can be fixed by wrapping the “sleep(100);” statement with a separate IF statement to repeat the logic only N-1 times:

   public BufferedReader tryOpenFile2(String fileName, int tryCount) throws IOException {
      Path path = FileSystems.getDefault().getPath(fileName);
      while (tryCount-- > 0) {
         System.out.println("Check if file exists. Try " + tryCount);
         if (Files.exists(path)) {
            return Files.newBufferedReader(path, Charset.forName("UTF-8"));
         }
         if (tryCount > 0) {
            sleep(100);
         }
      }
      return null;
   }

The re-try logic is very common in old and modern applications. Even though the implementation looks simple the bug rate of the logic is surprisingly high in practice.

Thank you for reading this post. Try to avoid described bug in your loop implementations. Remember about N and N-1 logic in the loops.