patternjavaModerate
Do they have the POWER?
Viewed 0 times
thetheyhavepower
Problem
I'm working on a city building simulation game, and I have reached the point where I need to check whether tiles are connected to a power source. I decided on a flood fill for this, since it seemed simple enough to implement. For reference, I used this flood fill algorithm, and I also found this question on code review about flood fill in java. My code is largely similar to the code in those links, but it is slightly different because I am not filling an image with color but rather marking tiles connected to power plants as
I know that recursion is dangerous, and the efficiency of the code may be suspect. With the map mostly filled up, this method gets called a couple of thousand times per cycle. With just a snaky path, about 200 times.
Here is an example to show that the algorithm is working:
The visible tiles are powered.
Here is the code:
isPowered.I know that recursion is dangerous, and the efficiency of the code may be suspect. With the map mostly filled up, this method gets called a couple of thousand times per cycle. With just a snaky path, about 200 times.
Here is an example to show that the algorithm is working:
The visible tiles are powered.
Here is the code:
private void calculatePower() {
for (Tile tile : this.tilesForSort) {
tile.hasBeenSearched = false;
}
for (Tile tile : this.tilesForSort) {
if (tile.getBuildingType() == IsoTileType.POWER) {
this.powerFill(tile.getPosition().x, tile.getPosition().y);
}
}
}
public void powerFill(int x, int y) {
if (this.isNotInsideGrid(x, y)) {
return;
}
Tile tile = this.levelData[x][y];
if (tile.hasBeenSearched || tile.zoneType.getLevel() = World.WORLD_WIDTH || y >= World.WORLD_HEIGHT;
}Solution
It seems that
On the other hand, a field like
You probably don't wake up in the morning thinking, "hm, it will be nice to give the
This field could be encapsulated within a
that takes
The advantage of this approach will be that you can reuse
This call is a bit tedious, with the duplicated
It would be good to create a helper
The name
Perhaps something like this might work well:
The idea is:
-
It is the responsibility of the
-
The FloodFill` class doesn't need to know anything else about the world, and doesn't need to know what "filling" really means
This gives you the flexibility to reuse the flood fill logic to other purposes, without polluting the rest of your game model with specific knowledge catering to the flood fill process.
Tile is a key element in your game model.On the other hand, a field like
Tile.hasBeenSearched sounds something technical, not something that belongs to conceptual model.You probably don't wake up in the morning thinking, "hm, it will be nice to give the
Tile class a hasBeenSearched attribute". This field is an implementation detail of the flood-fill algorithm, and I don't think it fits well into Tile. It will be better to use a Set searched.This field could be encapsulated within a
FloodFiller class,that takes
tilesForSort and returns a collection of tiles for which the caller can set isPowered to true.The advantage of this approach will be that you can reuse
FloodFiller for other purposes, say, to actually flood your city.This call is a bit tedious, with the duplicated
tile.getPosition():powerFill(tile.getPosition().x, tile.getPosition().y);It would be good to create a helper
powerFill(tile.getPosition()) that forwards to powerFill(int, int).The name
isNotInsideGrid is a bit clumsy. How about isOutsideGrid ?Perhaps something like this might work well:
public class FloodFill {
interface World {
Set getEligibleNeighbors(Position position);
}
interface Position {
}
public static Set floodFill(World world, Position start) {
Set searched = new HashSet<>();
floodFill(world, searched, start);
return searched;
}
private static void floodFill(World world, Set searched, Position position) {
if (searched.contains(position)) {
return;
}
searched.add(position);
for (Position neighbor : world.getEligibleNeighbors(position)) {
floodFill(world, searched, neighbor);
}
}
}The idea is:
-
It is the responsibility of the
World implementation to return the valid neighbors of a position that are eligible for filling:- x +- 1, y +- 1, or whatever coordinate system you use
- Excluding positions outside of the world's boundary
- Excluding positions where
zoneType.getLevel()
- Any other custom logic you might need
-
The FloodFill` class doesn't need to know anything else about the world, and doesn't need to know what "filling" really means
This gives you the flexibility to reuse the flood fill logic to other purposes, without polluting the rest of your game model with specific knowledge catering to the flood fill process.
Code Snippets
powerFill(tile.getPosition().x, tile.getPosition().y);public class FloodFill {
interface World {
Set<Position> getEligibleNeighbors(Position position);
}
interface Position {
}
public static Set<Position> floodFill(World world, Position start) {
Set<Position> searched = new HashSet<>();
floodFill(world, searched, start);
return searched;
}
private static void floodFill(World world, Set<Position> searched, Position position) {
if (searched.contains(position)) {
return;
}
searched.add(position);
for (Position neighbor : world.getEligibleNeighbors(position)) {
floodFill(world, searched, neighbor);
}
}
}Context
StackExchange Code Review Q#92786, answer score: 10
Revisions (0)
No revisions yet.