###Overall
Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.
###Overall
I think you have too much code in the main method. You should split that into more digestible segments. Perhaps some more methods in your GameLogic
class (which probably should be split into one Game
class and one GameBot
/ Player
class). (At least) one method for determining the next move, one method for choosing the proper action based on the current tile, etc..
###Improving your bot
Assuming the gold never moves once it is placed, you could use a Set
of all the tiles you have visited. You could then try to not move to a tile you have already visited.
The tiles themselves could be represented as a Point
class that contains x
and y
, or it could be represented as an int if you do a little mathematics to represent a column and row by one single int (I do recommend the Point
class though). Or you could make a Tile
class, since each tile also has some content (wall, exit, gold, empty space...)
###Random
Random rn = new Random();
int min = 0;
int max = 3;
int n = max - min + 1;
int i = rn.nextInt(n);
- Since
min
is zero, there's no need to do- min
. - As max is a constant, it'd be easier to declare max as 4 from the start rather than using
+ 1
- As
n
only is used once, you could just useint i = rn.nextInt(4);
- Random objects are meant to be re-used. Initialize your Random object outside the loop. This is because randomization will be different when creating the object over and over again.
###Moving
You can move in four directions, this is telling me that you should use a Direction enum and replace your four methods with one move(Direction4 dir);
By using this enum, you could also simplify your random process:
Direction4 chosenDirection = Direction4.values()[rn.nextInt(Direction4.values().length)];
This line grabs all the possible directions and randomizes an int from 0 (inclusive) to length (exclusive) and grabs the direction with that index.
###When something goes wrong
System.out.println("SOMETHING WENT WRONG!!!");
I can assure you, this will never happen. And even if it would, Exceptions are meant to be for... well, exceptions. throw new AssertionError("Something went wrong.");
is what I would have done here. Or rather... I would have removed the entire line, as it will never happen. Especially when you use the Direction4 approach.
###Spacing
This is one of the things I mention most often in my reviews probably, but please:
}else if(i == 1){
change to
} else if (i == 1) {
It makes the code more readable with proper spacing.