I have three methods with duplicated code. The first two methods are nearly completely duplicated. The third one differs a bit as for fires more information should be drawn.
I want to remove this duplicated code and thought about the template method pattern using inner classes. Is this the right way to go or is there a better solution?
private void drawWaterSupplies(Graphics g) {
double hScale = getWidth() / (double) groundMap.getWidth();
double vScale = getHeight() / (double) groundMap.getHeight();
int imageOffsetX = waterSupplyImage.getWidth() / 2;
int imageOffsetY = waterSupplyImage.getHeight() / 2;
for (Location l : groundMap.getWaterSupplyLocations()) {
int x = (int) (l.getX() * hScale);
int y = (int) (l.getY() * vScale);
g.drawImage(waterSupplyImage, x - imageOffsetX, y - imageOffsetY,
null);
}
}
private void drawEnergySupplies(Graphics g) {
double hScale = getWidth() / (double) groundMap.getWidth();
double vScale = getHeight() / (double) groundMap.getHeight();
int imageOffsetX = energySupplyImage.getWidth() / 2;
int imageOffsetY = energySupplyImage.getHeight() / 2;
for (Location l : groundMap.getEnergySupplyLocations()) {
int x = (int) (l.getX() * hScale);
int y = (int) (l.getY() * vScale);
g.drawImage(energySupplyImage, x - imageOffsetX, y - imageOffsetY,
null);
}
}
private void drawFires(Graphics g) {
double hScale = getWidth() / (double) groundMap.getWidth();
double vScale = getHeight() / (double) groundMap.getHeight();
int imageOffsetX = fireImage.getWidth() / 2;
int imageOffsetY = fireImage.getHeight() / 2;
for (Fire fire : groundMap.getFires()) {
Location l = fire.getLocation();
int x = (int) (l.getX() * hScale);
int y = (int) (l.getY() * vScale);
g.drawImage(fireImage, x - imageOffsetX, y - imageOffsetY, null);
// TODO: draw status bar showing state of fire below
}
}
It seems to me your collection of objects (Fire, WaterSupply etc.) aren't as clever as they should be. Ideally you should be able to say:
for (Fire f : groundMap.getFires()) {
f.draw(g);
}
and each object would be able to locate itself (using its location), size itself (since a Fire knows it's going to use a FireImage etc.) and draw itself onto the provided Graphics object.
To take this further, I would expect to pass a Graphics object to your groundMap thus:
groundMap.drawFires(g);
The idea is that in OO you shouldn't ask objects for their details and then make decisions. Instead you should tell objects to do things for you.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With