-
-
Notifications
You must be signed in to change notification settings - Fork 691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tests two bucket #2877
base: main
Are you sure you want to change the base?
Update tests two bucket #2877
Changes from 9 commits
c2b862d
16b0389
45e971d
a891663
ca31ed3
c1e8519
bba5d41
10230fc
4a168d9
574b207
916eef4
0a0b731
3bacb8c
9e47280
9935ee5
81210a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,149 +1,140 @@ | ||
import java.util.ArrayList; | ||
import java.util.Objects; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
class TwoBucket { | ||
|
||
private class State { | ||
int moves; | ||
int bucketOne; | ||
int bucketTwo; | ||
|
||
State (int moves, int bucketOne, int bucketTwo) { | ||
this.moves = moves; | ||
this.bucketOne = bucketOne; | ||
this.bucketTwo = bucketTwo; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
State otherState = (State) o; | ||
return this.moves == otherState.moves && | ||
this.bucketOne == otherState.bucketOne && | ||
this.bucketTwo == otherState.bucketTwo; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(moves, bucketOne, bucketTwo); | ||
} | ||
} | ||
|
||
private State finalState; | ||
|
||
private int bucketOneCap; | ||
private int bucketTwoCap; | ||
private int desiredLiters; | ||
private String startBucket; | ||
|
||
private int totalMoves = Integer.MAX_VALUE; | ||
private String finalBucket = ""; | ||
private int otherBucket = Integer.MAX_VALUE; | ||
|
||
private final int bucketOneCap; | ||
private final int bucketTwoCap; | ||
private final int desiredLiters; | ||
|
||
TwoBucket(int bucketOneCap, int bucketTwoCap, int desiredLiters, String startBucket) { | ||
fillBuckets( | ||
startBucket.equals("one") ? bucketOneCap : 0, | ||
bucketOneCap, | ||
startBucket.equals("two") ? bucketTwoCap : 0, | ||
bucketTwoCap, | ||
desiredLiters, | ||
1, | ||
startBucket | ||
); | ||
|
||
this.bucketOneCap = bucketOneCap; | ||
this.bucketTwoCap = bucketTwoCap; | ||
this.desiredLiters = desiredLiters; | ||
this.startBucket = startBucket; | ||
|
||
finalState = computeFinalState(); | ||
} | ||
|
||
private ArrayList<State> getAdjacentStates (State state) { | ||
ArrayList<State> adjacentStates = new ArrayList<State>(); | ||
|
||
//Empty bucket one | ||
adjacentStates.add(new State(state.moves + 1, 0, state.bucketTwo)); | ||
|
||
//Empty bucket two | ||
adjacentStates.add(new State(state.moves + 1, state.bucketOne, 0)); | ||
|
||
//Fill bucket one | ||
adjacentStates.add(new State(state.moves + 1, bucketOneCap, state.bucketTwo)); | ||
private List<Map.Entry<Integer, Integer>> statesReached = new ArrayList<>(); | ||
|
||
private void fillBuckets( | ||
int bucketOne, | ||
int bucketOneCap, | ||
int bucketTwo, | ||
int bucketTwoCap, | ||
int desiredLiters, | ||
int actionsTaken, | ||
String startingBucket | ||
) { | ||
if (startingBucket.equals("one") && bucketOne == 0 && bucketTwo == bucketTwoCap | ||
|| startingBucket.equals("two") && bucketTwo == 0 && bucketOne == bucketOneCap | ||
|| statesReached.contains(Map.entry(bucketOne, bucketTwo)) | ||
|| bucketOne > bucketOneCap | ||
|| bucketTwo > bucketTwoCap) { | ||
return; | ||
} | ||
|
||
if (bucketOne == desiredLiters) { | ||
if (actionsTaken < totalMoves) { | ||
this.totalMoves = actionsTaken; | ||
this.finalBucket = "one"; | ||
this.otherBucket = bucketTwo; | ||
} | ||
return; | ||
} | ||
if (bucketTwo == desiredLiters) { | ||
if (actionsTaken < totalMoves) { | ||
this.totalMoves = actionsTaken; | ||
this.finalBucket = "two"; | ||
this.otherBucket = bucketOne; | ||
} | ||
return; | ||
} | ||
|
||
//Fill bucket two | ||
adjacentStates.add(new State(state.moves + 1, state.bucketOne, bucketTwoCap)); | ||
statesReached.add(Map.entry(bucketOne, bucketTwo)); | ||
|
||
//pour from bucket one to bucket two | ||
if (state.bucketOne + state.bucketTwo > bucketTwoCap) { | ||
adjacentStates.add(new State(state.moves + 1, | ||
state.bucketOne - (bucketTwoCap - state.bucketTwo), | ||
bucketTwoCap)); | ||
} else { | ||
adjacentStates.add(new State(state.moves + 1, 0, state.bucketOne + state.bucketTwo)); | ||
if (bucketOne != 0) { | ||
fillBuckets(0, bucketOneCap, bucketTwo, bucketTwoCap, desiredLiters, actionsTaken + 1, startingBucket); | ||
} | ||
|
||
//pour from bucket two to bucket one | ||
if (state.bucketTwo + state.bucketOne > bucketOneCap) { | ||
adjacentStates.add(new State(state.moves + 1, | ||
bucketOneCap, | ||
state.bucketTwo - (bucketOneCap - state.bucketOne))); | ||
} else { | ||
adjacentStates.add(new State(state.moves + 1, state.bucketTwo + state.bucketOne, 0)); | ||
if (bucketTwo != 0) { | ||
fillBuckets(bucketOne, bucketOneCap, 0, bucketTwoCap, desiredLiters, actionsTaken + 1, startingBucket); | ||
} | ||
|
||
return adjacentStates; | ||
} | ||
|
||
private boolean isValid(State state) { | ||
if (state.bucketOne == bucketOneCap && state.bucketTwo == 0 && startBucket.equals("two")) { | ||
return false; | ||
} else if (state.bucketOne == 0 && state.bucketTwo == bucketTwoCap && startBucket.equals("two")) { | ||
return false; | ||
fillBuckets( | ||
bucketOneCap, bucketOneCap, bucketTwo, bucketTwoCap, desiredLiters, | ||
actionsTaken + 1, startingBucket | ||
); | ||
fillBuckets( | ||
bucketOne, bucketOneCap, bucketTwoCap, bucketTwoCap, desiredLiters, | ||
actionsTaken + 1, startingBucket | ||
); | ||
|
||
if (bucketOne + bucketTwo <= bucketTwoCap) { | ||
fillBuckets( | ||
0, bucketOneCap, bucketTwo + bucketOne, bucketTwoCap, desiredLiters, | ||
actionsTaken + 1, startingBucket | ||
); | ||
} else { | ||
return true; | ||
fillBuckets( | ||
bucketOne + bucketTwo - bucketTwoCap, bucketOneCap, bucketTwoCap, bucketTwoCap, | ||
desiredLiters, actionsTaken + 1, startingBucket | ||
); | ||
} | ||
} | ||
|
||
private State computeFinalState() { | ||
ArrayList<State> paths = new ArrayList<State>(); | ||
|
||
State initialState; | ||
if (startBucket.equals("one")) { | ||
initialState = new State(1, bucketOneCap, 0); | ||
if (bucketOne + bucketTwo <= bucketOneCap) { | ||
fillBuckets( | ||
bucketOne + bucketTwo, bucketOneCap, 0, bucketTwoCap, desiredLiters, | ||
actionsTaken + 1, startingBucket | ||
); | ||
} else { | ||
initialState = new State(1, 0, bucketTwoCap); | ||
fillBuckets( | ||
bucketOneCap, bucketOneCap, bucketTwo + bucketOne - bucketOneCap, bucketTwoCap, | ||
desiredLiters, actionsTaken + 1, startingBucket | ||
); | ||
} | ||
} | ||
|
||
if (initialState.bucketOne == desiredLiters || initialState.bucketTwo == desiredLiters) { | ||
return initialState; | ||
private void checkIfImpossible() { | ||
boolean exceedsCapacity = desiredLiters > bucketOneCap && desiredLiters > bucketTwoCap; | ||
boolean invalidDivision = desiredLiters % gcd(bucketOneCap, bucketTwoCap) != 0; | ||
|
||
if (exceedsCapacity || invalidDivision) { | ||
throw new IllegalArgumentException("impossible"); | ||
} | ||
} | ||
|
||
paths.add(initialState); | ||
|
||
for (int i = 0; i < 10000; i++) { | ||
State currentState = paths.remove(0); | ||
ArrayList<State> adjacentStates = getAdjacentStates(currentState); | ||
for (State state : adjacentStates) { | ||
if (state.bucketOne == desiredLiters || state.bucketTwo == desiredLiters) { | ||
return state; | ||
} | ||
|
||
if (!paths.contains(state) && isValid(state)) { | ||
paths.add(state); | ||
} | ||
} | ||
private int gcd(int a, int b) { | ||
while (b != 0) { | ||
int temp = b; | ||
b = a % b; | ||
a = temp; | ||
} | ||
|
||
return null; | ||
return a; | ||
} | ||
|
||
int getTotalMoves() { | ||
return finalState.moves; | ||
checkIfImpossible(); | ||
return totalMoves; | ||
} | ||
|
||
String getFinalBucket() { | ||
if (finalState.bucketOne == desiredLiters) { | ||
return "one"; | ||
} else if (finalState.bucketTwo == desiredLiters) { | ||
return "two"; | ||
} else { | ||
return "No solution found in " + finalState.moves + " iterations!"; | ||
} | ||
checkIfImpossible(); | ||
return finalBucket; | ||
} | ||
|
||
int getOtherBucket() { | ||
if (getFinalBucket().equals("one")) { | ||
return finalState.bucketTwo; | ||
} else if (getFinalBucket().equals("two")) { | ||
return finalState.bucketOne; | ||
} else { | ||
return -1; | ||
} | ||
checkIfImpossible(); | ||
return otherBucket; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -2,6 +2,7 @@ | |||
import org.junit.jupiter.api.Test; | ||||
|
||||
import static org.assertj.core.api.Assertions.assertThat; | ||||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType; | ||||
|
||||
public class TwoBucketTest { | ||||
|
||||
|
@@ -75,4 +76,52 @@ public void testBucketOneSizeTwoBucketTwoSizeThreeStartWithOne() { | |||
assertThat(twoBucket.getOtherBucket()).isEqualTo(2); | ||||
|
||||
} | ||||
|
||||
@Disabled("Remove to run test") | ||||
@Test | ||||
public void testReachingGoalIsImpossible() { | ||||
|
||||
TwoBucket twoBucket = new TwoBucket(6, 15, 5, "one"); | ||||
|
||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getTotalMoves()) | ||||
.withMessage("impossible"); | ||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getFinalBucket()) | ||||
.withMessage("impossible"); | ||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getOtherBucket()) | ||||
.withMessage("impossible"); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that we have the
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I included the error message initially because the |
||||
|
||||
} | ||||
|
||||
@Disabled("Remove to run test") | ||||
@Test | ||||
public void testBucketOneSizeSixBucketTwoSizeFifteenStartWithOne() { | ||||
|
||||
TwoBucket twoBucket = new TwoBucket(6, 15, 9, "one"); | ||||
|
||||
assertThat(twoBucket.getTotalMoves()).isEqualTo(10); | ||||
assertThat(twoBucket.getFinalBucket()).isEqualTo("two"); | ||||
assertThat(twoBucket.getOtherBucket()).isEqualTo(0); | ||||
|
||||
} | ||||
|
||||
@Disabled("Remove to run test") | ||||
@Test | ||||
public void testGoalLargerThanBothBucketsIsImpossible() { | ||||
|
||||
TwoBucket twoBucket = new TwoBucket(5, 7, 8, "one"); | ||||
|
||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getTotalMoves()) | ||||
.withMessage("impossible"); | ||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getFinalBucket()) | ||||
.withMessage("impossible"); | ||||
assertThatExceptionOfType(IllegalArgumentException.class) | ||||
.isThrownBy(() -> twoBucket.getOtherBucket()) | ||||
.withMessage("impossible"); | ||||
|
||||
} | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is a bit odd to have to have to assert that we get an exception three times in the test (I was expecting only one assertion). What do you think about refactoring to assert the exception just once? In the csharp implementation, the
TwoBucket
class has ameasure
method that the tests calls. It returns a "Result" object containing the number of moves, the final bucket and the other bucket or throws an exception if the goal can be reached.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even I found it a bit odd to assert the exception three times while implementing the tests but couldn’t think of a better alternative. However, your suggestion of returning a
Result
object and handling everything within a single method is an excellent idea! This approach not only simplifies the implementation but also ensures the custom exception can be thrown effectively. Should I go ahead with this approach?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we can go ahead with this one as we both seem to agree it would be better this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we can go ahead with this one as we both seem to agree it would be better this way.