2

I'm building a board game type game in JavaFX. Different GameObjects have different ObjectTypes. I'm currently storing a few static Images and referencing them when I need a new ImageView.

public class GameObject extends ImageView {

private static final Image BG_IMG = new Image(RESPATH + "BG.png");
private static final Image NULL_IMG = new Image(RESPATH + "NULL.png");

private static final Image T2_IMG = new Image(RESPATH + "T2.png");
private static final Image T3_IMG = new Image(RESPATH + "T3.png");
private static final Image T4_IMG = new Image(RESPATH + "T4.png");
private static final Image T5_IMG = new Image(RESPATH + "T5.png");
private static final Image T6_IMG = new Image(RESPATH + "T6.png");

//ObjectType is an enum type
private final ObjectType OBJECT_ID;

private Image img;

public GameObject(ObjectType t) {
    OBJECT_ID = t;
    setImage();
    setImage(img);
}

private void setImage() {
    switch (OBJECT_ID) {
        case T2: img = (T2_IMG);
            break;
        case T3: img = (T3_IMG);
            break;
        case T4: img = (T4_IMG);
            break;
        case T5: img = (T5_IMG);
            break;
        case T6: img = (T6_IMG);
            break;
        case BG: img = (BG_IMG);
            break;
        default: img = NULL_IMG;
            break;
    }
}

I was wondering if this was a good way to implement the Images since I will be using the same Image for multiple ImageViews.

The other way I would implement this:

private void setImage() {
    img = new Image(RESPATH + OBJECT_ID.name() + ".png")
}

If I did it this way, I'd just get rid of the static Images.

paragon
  • 23
  • 5
  • Reusing the same image instance in multiple image views will save memory (and potentially other resources). There are probably more elegant ways to do it than the one you have shown, but this is a viable approach, and should perform well. – James_D Jul 31 '17 at 11:56

1 Answers1

1

Your first approach takes up more memory since you hold all the images in memory, but they are available for retrieval quickly. The second approach loads the image on-the-fly so you save on memory but not pay for it in processing time.

This is the standard caching trade off. You have to ask yourself how often the images are loaded/needed, how fast you need this to happen, and how much memory you can spare. If you can spare the memory, I would use a Map<ObjectType, Image> which is cleaner and maybe faster (see comments) than the switch lookup if you use a HashMap.

user1803551
  • 12,965
  • 5
  • 47
  • 74
  • 1
    I'm not 100% sure a map is faster than a switch. If you use the wrong map type, it's probably slower... It would definetly be cleaner of course... – fabian Jul 31 '17 at 15:03
  • @fabian Well, switching is O(n) and `HashMap`'s `get` is usually O(1), worst case O(n). So that's what I was referring to. I edited that in. Otherwise, I don't know how the OP can `switch` on `ObjectType` anyway. – user1803551 Jul 31 '17 at 15:10
  • 1
    Nope, it's compiled to a `lookupswitch` which provides better-than-linear-time search... Also for such a small numer of cases the overhead of using a map is probably worse than doing a search on such a small case list... – fabian Jul 31 '17 at 15:30
  • @fabian Ah, didn't know about the `lookupswitch` compilation. The map does get better with more entries, but I don't know how many images the OP really has. I updated the answer again. – user1803551 Jul 31 '17 at 15:33
  • Your first paragraph seems to assume there is just one image view and the OP is switching the image in it. I assumed in my comment there were many image views, potentially with some of them showing the same image. Obviously if the latter is true then caching wins. – James_D Jul 31 '17 at 15:39
  • @user1803551 "I don't know how the OP can switch on `ObjectType` anyway": presumably it's an `enum`. – James_D Jul 31 '17 at 15:40
  • @user1803551 Should my `HashMap` be implemented in the `ObjectType`(enum) file or the `GameObject`? I have a few more images than the ones shown in my question Or could the images simply be a field in the enum? – paragon Aug 01 '17 at 19:18