3

我有三种重复代码的方法。前两种方法几乎完全重复。第三个有点不同，因为应该绘制更多关于火灾的信息。

我想删除这个重复的代码并考虑使用内部类的模板方法模式。这是正确的方法还是有更好的解决方案？

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
    }
}
4

8 回答 8

5

在我看来，您的对象集合（Fire等WaterSupply）并不像应有的那样聪明。理想情况下，您应该能够说：

for (Fire f : groundMap.getFires()) {
   f.draw(g);
}

并且每个对象都能够定位自己（使用它的位置）、调整自身大小（因为 aFire知道它将使用 aFireImage等）并将自己绘制到提供的 Graphics 对象上。

为了更进一步，我希望将一个Graphics对象传递给你的groundMap：

groundMap.drawFires(g);

这个想法是，在 OO 中，您不应该询问对象的详细信息然后做出决定。相反，您应该告诉对象为您做事。

于 2012-10-02T13:05:44.977 回答
1

我将委托给另一个方法并为火、水和能源创建一个超类。这个超类将包含所有公共属性。例如getLocation()

例如

private void drawEverything(Graphics g, Image im, List<? extends SuperClassOfFireEtc> list, double w, double h) {
    double hScale = getWidth() / w;
    double vScale = getHeight() / h;

   int imageOffsetX = im.getWidth() / 2;
   int imageOffsetY = im.getHeight() / 2;
   for (SuperClassOfFireEtc f : list) {
       Location l = f.getLocation();
       int x = (int) (l.getX() * hScale);
       int y = (int) (l.getY() * vScale);

       g.drawImage(im, x - imageOffsetX, y - imageOffsetY, null); 
   }

}

然后drawFire可以调用

 private void drawEverything(g, fireImage, groundMap.getFires(), groundMap.getWidth(), groundMap.getHeight()) {
于 2012-10-02T13:05:46.133 回答
1

怎么样：

private void drawImageAtLocations(Graphics g, Image i, Collection<Location> cl) {
    double hScale = getWidth() / (double) groundMap.getWidth();
    double vScale = getHeight() / (double) groundMap.getHeight();

    int imageOffsetX = i.getWidth() / 2;
    int imageOffsetY = i.getHeight() / 2;
    for (Location l : cl) {
        int x = (int) (l.getX() * hScale);
        int y = (int) (l.getY() * vScale);

        g.drawImage(i, x - imageOffsetX, y - imageOffsetY, null);
    }
}

前两个开箱即用：

drawImageAtLocations(g, waterSupplyImage, groundMap.getWaterSupplyLocations());
drawImageAtLocations(g, energySupplyImage, groundMap.getEnergySupplyLocations());

第三个有点混乱，但仍然比原来的短：

Set<Location> derp = new HashSet<Location>();
for (Fire fire : groundMap.getFires()) derp.add(fire.getLocation());
drawImageAtLocations(g, fireImage, derp);
// drawImageAtLocations(g, fireStatusBarImage, derp); // TODO blah blah
于 2012-10-02T13:14:31.247 回答
0

首先，您似乎可以有一个接收Graphics、aList<Location>和图像的通用方法，因此第一种和第二种方法可以提取该列表并调用。

第三种方法可以提取 Fire 的列表，然后创建一个对应的List<Location>. 然后使用新方法。

private void drawImages(Graphics g, List<Location> where, Image imageToDraw) {
...
}
于 2012-10-02T13:06:06.863 回答
0

我认为一个简单的解决方案是使用一种方法并传递图形和位置集合。

于 2012-10-02T13:06:56.993 回答
0

你可以有一个方法。

private void drawImageAtLocations(Graphics g, Image image, List<Location> locations) {
    double hScale = getWidth() / (double) groundMap.getWidth();
    double vScale = getHeight() / (double) groundMap.getHeight();

    int imageOffsetX = image.getWidth() / 2;
    int imageOffsetY = image.getHeight() / 2;
    for (Location l : locations) {
        int x = (int) (l.getX() * hScale);
        int y = (int) (l.getY() * vScale);

        g.drawImage(image, x - imageOffsetX, y - imageOffsetY, null);
    }
}

private void drawWaterSupplies(Graphics g) {
    drawImageAtLocations(g, waterSupplyImage, groundMap.getWaterSupplyLocations());
}
于 2012-10-02T13:07:11.330 回答
0

这就是我的提议

enum  Supplies {FIRE(fireImage), WATER(waterImage), ENERGY(energyImage); 
private Bitmap image;
Supplies(Bitmap image)
 {
  this.image = image
 }

 public getImage()
 {
  return image;
 }

} 

private void draw(Graphics g,Supplies supply) {
    double hScale = getWidth() / (double) groundMap.getWidth();
    double vScale = getHeight() / (double) groundMap.getHeight();

    int imageOffsetX = supply.getImage.getWidth() / 2;
    int imageOffsetY = supply.getImage.getHeight() / 2;
    for (Location location : groundMap.getLocations(supply)) {

        int x = (int) (location .getX() * hScale);
        int y = (int) (location .getY() * vScale);

        g.drawImage(supply.getImage, x - imageOffsetX, y - imageOffsetY, null);
    }
}

....您可以在 Map> 中保存的所有火、水、能源等位置，因此方法 getLocations(supply) 或多或少看起来像这样

List<Location> getLocations(Supplies supply)
{
return supplyMap.get(supply);
}

如果您想添加或删除耗材和事故，此解决方案可为您提供更大的灵活性

于 2012-10-02T13:19:56.813 回答
0

不是最短的选择，但它使代码更干净，更容易扩展：

enum YourEnum {
    WATER,
    ENERGY,
    FIRE;
}

private void draw(Graphics g, YourEnum type) {
    Bitmap bitmap = getRightBitmap(type);
    double hScale = getWidth() / (double)groundMap.getWidth();
    double vScale = getHeight() / (double)groundMap.getHeight();

    int imageOffsetX = bitmap.getWidth() / 2;
    int imageOffsetY = bitmap.getHeight() / 2;
    for (Location l : getLocations(type)) {
        int x = (int)(l.getX() * hScale);
        int y = (int)(l.getY() * vScale);

        g.drawImage(bitmap, x - imageOffsetX, y - imageOffsetY,
                null);
    }
}



private Bitmap getRightBitmap(YourEnum type) {
    switch (type) {
        case WATER:
            return waterSupplyImage;
        case ENERGY:
            return waterSupplyImage;
        case FIRE:
            return fireImage;
    }
}

private Collection<Location> getLocations(YourEnum type) {
    switch (type) {
        case WATER:
            return groundMap.getWaterSupplyLocations();
        case ENERGY:
            return groundMap.getEnergySupCollections();
        case FIRE:
            Collection<Location> locations = new ArrayList<Location>();
            for (Fire fire : groundMap.getFires()) {
                locations.add(fire.getLocation());
            }
            return locations;
    }
}
于 2012-10-02T13:21:56.690 回答