public ArrayList<DirectionContainer> read_input(String filename) {
  // Somehow reads files: don't change!
  ArrayList<DirectionContainer> arr = new ArrayList<>();

  ...
  
  return arr;
}

/**
* Reads an input file according to the specification of each line
* being a word and a number separated by a whitespace
*
* @param filename the input file name to be read
* @return An array of Movement Directions
*/
public ArrayList<DirectionContainer> read_input(String filename) {

    // List of direction steps to be taken in type-safe form
    ArrayList<DirectionContainer> arr = new ArrayList<>();

    // Use BufferedReader for performance
    // See: https://docs.oracle.com/javase/8/docs/api/java/io/BufferedReader.html
    try (BufferedReader reader = new BufferedReader(new FileReader(filename))) {
        String line;
        // While reading till EOF
        while ((line = reader.readLine()) != null) {

            // The first word is the direction (in String)
            // and the second word is the number of steps
            String[] command = line.split("\\s+");
            arr.add(new DirectionContainer(command[0], Integer.parseInt(command[1])));
        }
    } catch (IOException x) {
        System.err.format("Input file: failed reading %s%n", x);
    }

    return arr;
}

While one should strive for self-documenting code, care should be taken in writing judicious comments. Good comments increases readability, reduces bugs (because the assumptions of pre/post conditions have already been documented), and readier for change.

The code with bad style does not use any comments for context for the reader, and the single comment is not helpful to the reader. In contrast, the below example uses the following principles:

1. Specification in the form of javadoc, which appears above a function or above a class and documents the behaviour of the function/class.
2. Writing the reasoning of some design choices in the code (such as using BufferedReader), although this can also be mentioned separately in a report.
3. Specifies the source of a piece of code that was copied/adapted/used in reasoning. Again, we see that the reasoning for the use of BufferedReader.
4. Bad and unnecessary comments (for example literal meaning of each statement) is not given. One should understand that the reader has the proficiency to read Java syntax.

switch (md.getDirection()) {
  case "forward" -> x += md.getSteps();
  case "backward" -> x -= md.getSteps();
  case "down" -> y += md.getSteps();
  case "up" -> y -= md.getSteps();
  default -> throw new IOException("Bad Input");
}

// In Submarine.calculatePosition

switch (md.getDirection()) {
  case FORWARD -> x += md.getSteps();
  case BACKWARD -> x -= md.getSteps();
  case DOWN -> y += md.getSteps();
  case UP -> y -= md.getSteps();
}

Failing fast means that code should reveal its bugs as early in development as possible. Static languages such as Java provide reasonable type support as compared to dynamically typed languages like Python, which is more prone to runtime errors. In the example given above, after getting the input, we assume that it is sanitized and store it as a new enum type in Java, such as when a switch statement is run, we can be assured that most of the input would already be validated and corruption in function exection would not happen.

s.read_input("inputs/input.txt");

public static final String INPUT_FILE = "inputs/input.txt";
...
s.read_input(INPUT_FILE);

According to wikipedia, Magic Numbers are unique values with unexplained meaning or multiple occurrences which could (preferably) be replaced with named constants.

In the example given above, we don’t know what is “inputs/input.txt” just from the use of it. It is generally better to rename it as INPUT_FILE within the respective class, so as to make it easier to change (as constants - generally noted by the final keyword in Java by convention, need to change in the future). Note that we have defined INPUT_FILE as static to save memory.

public ArrayList<DirectionContainer> read_input(String filename, ArrayList<DirectionContainer> arr) {

  
  ...
  // Make operations with arr
  ...
  
  return arr;
}

public ArrayList<DirectionContainer> read_input(String filename) {

  // Define arraylist locally
  ArrayList<DirectionContainer> arr = new ArrayList<>();
  ...
  // Make operations with arr
  ...


    return arr;
}

In the bad example given above, the parameter arr is reused to compute a very different value compared from the beginning (by appending) – the return value of the function. Instead, it would be better to make this a pure function, with only one parameter fileName and defining arr in the local scope of the function.

Try not to reuse parameters and reuse variables. Their declaration does not carry a heavy overhead in programming, and minimalizating change of state would also tend to make a function with less/no side-effects. Embedding functional programming principles in Java would have the properties of safety-from-bugs and ready-to-change.

public static void main(String[] args) {
  Question q = new Question();
  ArrayList<DirectionContainer> md = q.read_input(INPUT_FILE);
  int fd = s.fDive(s.fPos(movementDirections));
  System.out.printf("Final multiplication position relative to origin: %d", fd);
}

// Read the input file and calculate the final dive
public static void main(String[] args) {
  Submarine s = new Submarine();
  ArrayList<DirectionContainer> movementDirections = s.read_input(INPUT_FILE);
  Position finalPos = s.calculatePosition(movementDirections);
  int finalDive = s.calculateDive(finalPos);
  System.out.printf("Final multiplication position relative to origin: %d", finalDive);
}

Good function/variables names are generally long and self-descriptive in nature. For example, instead of:

int tmp = 3600; // tmp is the number of seconds in an hour 

we can write:

int secondsPerHour = 3600;

Java-like languages also use snake-case naming to distinguish global constants (as seen from INPUT_FILE). But the local variables declared inside the function are read in camelCase notion (such as movementDirections).

Effectively Naming Software Thingies and Robert Martin’s Clean Code (Chapter 2) is highly recommended for best code practices in OOP languages.

ArrayList<DirectionContainer> md = q.read_input("inputs/input.txt", new ArrayList<>());

// Think about what would happen in multiple arguments
ArrayList<DirectionContainer> md = q.read_input("inputs/input.txt",
                                                new ArrayList<>());

Having consistent style guidelines (not too restrictive though) can help maintain code more easily (easy to refactor / increasing readability / etc). It is followed by the following principles:

1. IntelliJ has an auto code formatter invoked by Ctrl + Alt + Shift + L which helps in using constistent identation / whitespaces in code.
2. Putting linebreaks and appropriate spaces if the function signature is too large (like in the example provided above).
3. Using spaces over tabs (although holy wars have been fought over this). See [2]

// In Question.java
public static int x, y;

// In Position.java
public class Position {

  private int x, y;

  public Position(int x, int y) {
    this.x = x;
    this.y = y;
  }

  ...
  // getters and setters

From [3]:

Static variables are generally considered bad because they represent global state and are therefore much more difficult to reason about. In particular, they break the assumptions of object-oriented programming (OOP). In OOP, each object has its own state, represented by instance (non-static) variables. Static variables represent state across instances which can be much more difficult to unit test. This is mainly because it is more difficult to isolate changes to static variables to a single test.

If one is going to declare a global variable, one should aspire to make it immutable (to be used as an environment variable / global constant). This can be done by using the final keyword.

public void calculateDive(Position p) {
  System.out.println(Math.abs(x * y));
}

public Position calculateDive(Position p) {
  // Return value instead of printing it within the function
  return Math.abs(p.getX() * p.getY());
}

In the bad example provided above, calculateDive() sends the string to standard output, thereby limited the exposure of usign the result in other contexts than what is present in the function scope. If this is somewhat needed in the future due to less maintainability, the part of the codes using this function would have to rewritten. Hence, in the below example calculateDive returns the Integer result to be potentially used in further computer / seen by the programmer.

In general, only the highest-level parts of a program should interact with the human user or the console. Lower-level parts should strive to be pure functions (without side-effects). The sole exception here is debugging output, which can of course be printed on standard output. But that kind of output should not reach the design of production code, only a part of how you debug your message.

public void calculatePosition(ArrayList<DirectionContainer> movementDirections) {

  if (movementDirections.size() == 0) System.out.println("0");

  int x = 0;
  int y = 0;

  for (DirectionContainer md : movementDirections) {
      switch (md.getDirection()) {
          case FORWARD -> x += md.getSteps();
          case BACKWARD -> x -= md.getSteps();
          case DOWN -> y += md.getSteps();
          case UP -> y -= md.getSteps();
  }

  ...
}

public void calculatePosition(ArrayList<DirectionContainer> movementDirections) {


  // Remove special case handling
  
  int x = 0;
  int y = 0;

  for (DirectionContainer md : movementDirections) {
      switch (md.getDirection()) {
          case FORWARD -> x += md.getSteps();
          case BACKWARD -> x -= md.getSteps();
          case DOWN -> y += md.getSteps();
          case UP -> y -= md.getSteps();
  }

  ... 
}

The starting if statement in the bad code example is unnecesary. It it were ommitted, and the movementDirections array is empty, then the block in for loop is not run and we get the same expected output. In face, handling this special case separately has led to a potential bug - a difference in the way an empty array is handled, compared to a non-empty array that happens to have no direction instructions to it.

Hence, actively resist the temptation to handle special cases separately. Tackle the general case first, and see whether all the scenarios could be handled by it’s current version / very slight modification of it.

Write a clean, simple, general-case algorithm first, and optimize it later (if possible through special cases), only if there is evidence that it would actually help.