Higher performance reading shorts from binary file

I just got rejected at the tech interview of a job application. They gave me the following exercise for me to complete:

They gave me a binary file with 10 million pairs of 16-bit signed integers. From those 10 million, I had to find the 10 closest points to (-200, 300) and the 20 furthest points to (1000, 25).

I finished my implementation and I created a zip file with my "src" directory (I did it using IntelliJ IDEA). Since I could not ship the binary file too, because it was too big, I made a PATH_TO_FILE constant so they could write their own path to file.

They rejected me on the basis that I didn't include any tests (which I should have, in retrospect) and that my packing and commenting didn't conform to their standards.

My question is, if you can help me identify:

• How to improve the code's performance (it takes 3 seconds to run on my machine).

• How I could have "packed and commented" my code better.

• Any implementation tips that you can give me.

I created a command-line program that prints the answer to the console, which consisted of these two classes:

Main class:

```
package com.company;

import java.awt.*;
import java.io.*;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Comparator;

public class Main {

//TODO: Please, assign the variable below to the actual path to the file in your filesystem
static String FILENAME = "path/to/file";

private static final int OFFSET = 0;
private static final int LENGTH = 2;

// We declare the points that will be used in the comparison
static Point mPoint1 = new Point(-200,300);
static Point mPoint2 = new Point(1000,25);
// Params for amount of nodes we need to retrieve
static int CLOSEST_NODES_NUMBER = 10;
static int FURTHEST_NODES_NUMBER = 20;

// User feedback strings
static String START = "Computing the desired nodes, please wait...";
static String SEPARATOR = "\n**\n";
stat

The good:

• You defined static variables for everything, making it more easy to change the values.

• You created your own queue simplifying the logic of the algorithm

The bad:

• You put almost all of your code in the main method.

• (Related to point 1) You didn't define a method to return the points needed

• (Related to point 1) You didn't separate concerns (showing the points from displaying them)

• You swallow an exception and you do not rethrow it. Java (and almost every other language that is aware of exceptions)

already displays the stack strace when an exception is raised, there wasn't a need for you to do the same

What you missed:

• Scanner would make your job a lot easier since it provides a nextShort method

• Try-with-resources is now the prefered way to clean-up

• If the file has 0 elements or any odd number of numbers it will print an error

• You were not consistent on the naming and declaration of your variables namely mPoint1 and mPoint2

• Some of your messages do not adjust to different points if needed

I would redo it as follows:

//TODO: Please, assign the variable below to the actual path to the file in your filesystem
private static String FILENAME = "path/to/file";

private static final int OFFSET = 0;
private static final int LENGTH = 2;

// We declare the points that will be used in the comparison
private static Point POINT_1 = new Point(-200,300);
private static Point POINT_2 = new Point(1000,25);
// Params for amount of nodes we need to retrieve
private static int CLOSEST_NODES_NUMBER = 10;
private static int FURTHEST_NODES_NUMBER = 20;

// User feedback strings
private static String START = "Computing the desired nodes, please wait...";
private static String SEPARATOR = "\n****************************************************\n";
private static String END = "Thanks for your patience, here are your results: " + SEPARATOR;
private static String COMPUTATION_TIME = "The process took %s milliseconds";
private static String PRINT_CLOSEST = SEPARATOR +  String.format("These are the closest nodes to the point %d,%d", POINT_1.x, POINT_1.y) + SEPARATOR;
private static String PRINT_FURTHEST = SEPARATOR + String.format("These are the furthest nodes to the point %d,%d", POINT_2.x, POINT_2.y) + SEPARATOR;
private static String NODE_PRESENTATION = "Node n° %d : %s  with distance to the target node: %f\n";

public static LimitedPriorityQueue[] getClosestAndFurthestPoints(String file, Point pointClose, int nClosest, Point pointFurther, int nFurther){

    /*Prepare queues with a custom comparator, comparing the distance relative to `pointClose` and `pointFurther` respectivly */
    LimitedPriorityQueue closestQueue = new LimitedPriorityQueue<>(nClosest,
        new Comparator() {
            @Override
            public int compare(Point firstPoint, Point secondPoint) {
                return firstPoint.distance(pointClose) > secondPoint.distance(pointClose) ? -1 : 1;
            }
    });

    LimitedPriorityQueue furthestQueue = new LimitedPriorityQueue<>(nFurther,
        new Comparator() {
            @Override
            public int compare(Point firstPoint, Point secondPoint) {
                return firstPoint.distance(pointFurther) []{
        closestQueue,
        furthestQueue
    };
}

public static void main(String[] args) throws IOException {
    System.out.println(START);
    long now = System.currentTimeMillis();

    LimitedPriorityQueue[] allPoints = getClosestAndFurthestPoints(FILENAME, POINT_1, CLOSEST_NODES_NUMBER, POINT_2, FURTHEST_NODES_NUMBER);

    long elapsed = System.currentTimeMillis() - now;
    System.out.printf(COMPUTATION_TIME, elapsed);

    LimitedPriorityQueue closestQueue = allPoints[0];
    System.out.println(PRINT_CLOSEST);
    for (int i = CLOSEST_NODES_NUMBER; i > 0; i--) {
        Point node = closestQueue.poll();
        System.out.printf(NODE_PRESENTATION, i, node, node.distance(POINT_1));
    }

    LimitedPriorityQueue furthestQueue = allPoints[1];
    System.out.println(PRINT_FURTHEST);
    for (int i = FURTHEST_NODES_NUMBER; i > 0; i--) {
        Point node = furthestQueue.poll(); System.out.printf(NODE_PRESENTATION, i, node,n ode.distance(POINT_2));
    }
}

StackExchange Code Review Q#129290, answer score: 4