I think my Bridge's foundation is too fragile

I've just started a new class learning Java, and since it's been a long time since I've programmed in it I thought I'd put up my first project to get back in the swing of things. Here is my first assignment:


Welcome to the Bridge Crossing Problem. Person Pn can cross the bridge
in n minutes. Only one or two persons can cross at a time because it
is dark, and the flashlight must be taken on every crossing. When two
people cross, they travel at the speed of the slowest person. Devise a
sequence of crossings so that all four people get across the bridge in
no more than 17 minutes.

So I built a system of classes to handle all of this, these two here are the base two. How can I improve this code? Any new Java 8 features I'm not taking advantage of?

Position.java:

package bridge;

/**
 * An enumerated type whose values indicate sides of the bridge in the bridge problem.
 * Refer to these values using "Position.EAST" and "Position.WEST"
 */
public enum Position { EAST, WEST }

BridgeState.java:

```
package bridge;

/**
* This class represents states of the Bridge Crossing problem.
* It creates new bridge states, tests states for equality,
* and produces string representations of them.
* @author your name here
*/
public class BridgeState {

private final Position p1Location;
private final Position p2Location;
private final Position p5Location;
private final Position p10Location;
private final Position flashLocation;
private final int time;
/**
* Creates a new bridge state.
* Besides storing the positions of the persons and flashlight, a
* bridge state should also store the time taken so far to cross in
* integer minutes.
* @param p1Position position of the person who can cross in 1 minute
* @param p2Position position of the person who can cross in 2 minutes
* @param flashlightPosition position of the flashlight
* @param p5Position position of the person who can cross in 5

Let's start with a few basic findings. I know you're mainly aiming at Java 8 features, but maybe I can follow up with more later on when I have more time. I think it might also be interesting to see how this BridgeState class is used later.

Code Style

-
Your bracket positions are inconsistent. In some places you are
putting them on the same line (public String toString() {), in some
places you are putting them on the next line (public Position getFlashlightPosition()).

-
Your field is e.g. named p1Location, but your getter is named
getP1Position (same goes for the others). Usually getters are named after their corresponding fields (i.e. getP1Location).

-
At first glance I'm not a fan of your p1Location, p2Location, etc variables. It might be more beautiful to use something like

public class Person {
    int timeNeeded;
    Position position;
}

although I haven't thought about this in detail yet. On the one hand it makes your class more extensible, on the other hand the problem is quite well defined and unlikely to have a dynamic number of people.

-
Why is the flashlightPosition interrupting the person locations (read as: why not p1Position, p2Position, p5Position, p10Position, flashlighPosition)?

-
String str = new String(); is creating a basically unused object. Just initialize str with your first content.

-
You should tag your toString method with @Override.

-
Your javadoc states that BridgeState "creates new bridge states", which I cannot see. I would assume more logic behind state creation than a mere constructor (although one can probably argue about that).

-
Your Position enum is probably not useful outside a BridgeState context, so you might as well make it an inner class.

Correctness

-
When overriding equals (even if slightly altered from the one in Object) also override hashCode. Always.
Consider the following example (made up, but it's one of the simplest ways to see the issue and it's not completely unrealistic):

import java.util.HashMap;
import java.util.Map;

public class BridgeStateTest {
    public static void main(String[] args) {
        Map map = new HashMap<>();
        BridgeState bridgeState1 = new BridgeState(Position.EAST, Position.EAST,
                Position.EAST, Position.EAST, Position.EAST, 0);
        BridgeState bridgeState2 = new BridgeState(Position.EAST, Position.EAST,
                Position.EAST, Position.EAST, Position.EAST, 0);
        map.put(bridgeState1, "foo");
        System.out.println(bridgeState1.equals(bridgeState2)); // true
        System.out.println(map.containsKey(bridgeState2)); // false
    }
}

For more information see e.g. this question on Stack Overflow.

-
Your equals method throws a NullPointerException for bridgeState.equals(null). While you are not strictly overriding Object.equals, you should try to follow the principle of least astonishment (or rename your method). I'd suggest to change the method parameter to indeed override the method from Object.

Other things: I like the immutability.

StackExchange Code Review Q#117535, answer score: 4