Minimum difference between numbers in a file

Question

I'm learning Java and I created a simple solution for the following problem:

Given a file [filename].txt such that each line has the format string, double, find the minimum possible difference between any two values and identify if it is less than an arbitrary value mindDist

package com.company;
import java.io.*;
import java.util.*;
public class Main {
 public static void main(String[] args) { try {
 Scanner sc = new Scanner(System.in);
 System.out.print("Please enter the name of the input file: ");
 Scanner inFile = new Scanner(new File(sc.nextLine()));
 sc.close();
 List<Double> l = new ArrayList<>();
 while (inFile.hasNextLine()) {
 String[] parts = inFile.nextLine().split(",");
 l.add(Double.parseDouble(parts[1]));
 }
 Collections.sort(l);
 double minDist = 5;
 double diff;
 double minDiff = Double.MAX_VALUE;
 for (int i = 1; i < l.size(); i++) {
 diff = l.get(i) - l.get(i - 1);
 if (diff < minDiff) minDiff = diff;
 }
 if (minDiff < minDist) System.out.println("The satellites are not in safe orbits.");
 else System.out.println("The satellites are in safe orbits.");
 if (l.size()!=1) System.out.println("The minimum distance between orbits (km): " + minDiff);
 inFile.close();
 } catch (NumberFormatException | FileNotFoundException | ArrayIndexOutOfBoundsException e) {
 System.err.println(e);
 }
 }
}

I am looking on advice on if there are any other possible errors / exceptions I may have missed, in addition to any way of increasing the efficiency / simplicity of my code.

• mdfst13: I posted a Meta question about an edit to this question and some other cosmetic issues. This should not block people from answering, as any proposed edits should not invalidate answers based on the code.

Answer 1

• You need to make sure you always close resources that can be closed. That means either a try-with-resources block or a try-finally block. You can’t just leave close outside one of these constructs because if an exception gets thrown the close method might not get called.

• Arguably, parsing the distances is easier to read using the stream API. You might not yet have learned that part of the language yet, in which case there’s nothing wrong with your loop except that inFile is never closed. Oh, no, it is, but waaaay too late. Keep the scope of the scanner as tight as possible.

• Likewise, the code for finding the minimum distance can be written as a stream. Credit to @AnkitSont.

• Variable names are really, really important. They should describe the information being pointed to. l is a terrible variable name because it doesn’t tell us anything. list would be a slight improvement, but distances would be much better.

• Along the same lines, there’s no reason to cut short variable name lengths. sc instead of scanner doesn’t save you anything, and it makes it harder on the reader because they have to go dig up what an sc is. minDist, diff and minDiffcan and should all be expanded out, and minDist might be better named safeDistance.

• Declare variables where they’re first used, and limit their scope as much as possible. This reduces the cognitive load on the reader. minDist can be declared right before your output. It could arguably also be a constant in your class (private static final double declared at the class level). diff can be declared inside the for loop.

• Even though the language allows curly braces to be optional for one-line blocks, it is widely regarded best practice that you always include curly braces. To do otherwise is inviting problems later when your code is modified.

• Be careful with conditional checks. All of your code will work correctly if there are zero distances in the file until you hit if (l.size() != 1). You really want to check if (l.size() > 1), right?

• Your catch block isn’t really doing anything. Uncaught exceptions would effectively be handled the same way - execution would terminate and the stack trace would be logged to System.err.

• It's nice to use final when you can to indicate that a variable isn't going to be reassigned. This also reduces the cognitive load of the reader.

If you were to apply all these changes to your code, it might look something like:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Scanner;
import java.util.stream.IntStream;
public final class Main {
 public static void main(final String[] args)
 throws IOException {
 final String filename;
 System.out.print("Please enter the name of the input file: ");
 try (final Scanner scanner = new Scanner(System.in)) {
 filename = scanner.nextLine();
 }
 final double[] distances =
 Files
 .lines(Paths.get(filename))
 .mapToDouble((line) -> Double.parseDouble(line.split(",")[1]))
 .sorted()
 .toArray();
 final double minimumDifference =
 IntStream
 .range(1, distances.length)
 .mapToDouble(i -> distances[i] - distances[i - 1])
 .min()
 .getAsDouble();
 final double safeDistance = 5;
 if (minimumDifference < safeDistance) {
 System.out.println("The satellites are not in safe orbits.");
 } else {
 System.out.println("The satellites are in safe orbits.");
 }
 if (distances.length > 1) {
 System.out.println("The minimum distance between orbits (km): " + minimumDifference);
 }
 }
}

Answer 2

• You can divide your main into smaller methods.
• The whole code shouldn't be wrapped inside a try/catch block. You should minimize the scope of your try block only where expect an Exception.
• Calculation of the minimum distance can be done using Stream.
• Try to have parentheses {} for if/else statements even when there's only one statement in it.
• Scanner should be closed as soon as its work is done, instead of closing at the very end. Or use try with resources.

• 5 can be extracted as a constant.

  class Main {
   private static final double MIN_DISTANCE = 5;
   public static void main(String[] args) {
   List<Double> satelliteDistances = getSatelliteDistancesFromFile();
   double minDiff = getMinimumDistanceBetweenSatellites(satelliteDistances);
   displayResults(satelliteDistances, minDiff);
   }
   private static double getMinimumDistanceBetweenSatellites(List<Double> satelliteDistances) {
   return IntStream.range(1, satelliteDistances.size())
   .mapToDouble(i -> satelliteDistances.get(i) - satelliteDistances.get(i - 1))
   .min().getAsDouble();
   }
   private static List<Double> getSatelliteDistancesFromFile() {
   List<Double> satelliteDistances = new ArrayList<>();
   try (Scanner fileScanner = getFileScanner()) {
   while (fileScanner.hasNextLine()) {
   String distanceAsString = getDistanceAsString(fileScanner);
   satelliteDistances.add(Double.parseDouble(distanceAsString));
   }
   }
   Collections.sort(satelliteDistances);
   return satelliteDistances;
   }
   private static String getDistanceAsString(Scanner fileScanner) {
   return fileScanner.nextLine().split(",")[1];
   }
   private static Scanner getFileScanner() {
   Scanner sc = new Scanner(System.in);
   System.out.print("Please enter the name of the input file: ");
   Scanner inFile = null;
   try {
   inFile = new Scanner(new File(sc.nextLine()));
   } catch (FileNotFoundException e) {
   e.printStackTrace();
   }
   sc.close();
   return inFile;
   }
   private static void displayResults(List<Double> satelliteDistances, double minDiff) {
   String safeOrbitResult = minDiff < MIN_DISTANCE ? "The satellites are not in safe orbits." : "The satellites are in safe orbits.";
   System.out.println(safeOrbitResult);
   if (satelliteDistances.size() != 1) {
   System.out.println("The minimum distance between orbits (km): " + minDiff);
   }
   }
  }