exercism / java

Exercism exercises in Java.
https://exercism.org/tracks/java
MIT License
689 stars 671 forks source link

need-for-speed: Niptpick: Tests allow for a batteryDrain that's larger than the available battery. #2796

Closed JoaquimEsteves closed 2 months ago

JoaquimEsteves commented 4 months ago

First of all, Thank you for your work on such a lovely website.

Back on topic, I have some nitpicks with the need-for-speed exercise.

Consider the following iterations 2 & 3

The tests will pass if we implement the batteryDrained method either like so

public boolean batteryDrained() {
    return battery <= 0;
}

Or, more correctly like so:

public boolean batteryDrained() {
    return battery - batteryDrain < 0;
}

Imagine a Car that is initialized with a batteryDrain larger than 50. Logically this car should only be driven once, as the remaining battery is not enough to execute a full drive.

I'd consider adding a simple test that would check this logic like so:

@Test
@Tag("task:XX")
@DisplayName("Should only drive once")
public void new_remote_control_car_that_can_only_drive_once() {
    var car = new NeedForSpeed(1, 99);
    car.drive();
    assertThat(car.batteryDrained()).isTrue();
    // Consider then testing the drive further
    car.drive();
    assertThat(car.distanceDriven()).isEqualTo(1)
    // Or maybe ensure that drive throws an exception
    // assertThatExceptionOfType(SomeException.class).isThrownBy(() -> car.drive());
}
manumafe98 commented 2 months ago

Thanks for opening the issue @JoaquimEsteves ! Yeah you're right checking in our current test suit we do not have a test like this. Im going to open a pr to add it!

JoaquimEsteves commented 2 months ago

If you'd appreciate a hand I'd love to help.

Exercism is lovely, and if I could contribute in anyway I'd be more than glad (even on such a tiny issue đŸ˜…)

manumafe98 commented 2 months ago

@JoaquimEsteves Yeah sure! go ahead with the PR then!