0

protected static final Map<String, Integer> m = new HashMap();

I have a question in regards to performance of using the above. I am creating a 2D Tile Engine for a simple RPG game. I am using the hash map above to store the name of a tile along with its respected color code (Ex: 0xff00ff21). Since this is a game, the code m.get("name");is called an enormous amount of times to check if a tile is updated or not. (Ex: The render method with my computer runs at about 850 times per second). Please also note, I made sure to declare the HashMap outside of any loops and that it is initialized via a method call(also static) through the constructor that m.put("name", value) inputs all the information.

1) Is using a HashMap in this way a good idea? Is there perhaps another way to go about this more efficiently.

2) Is using a static final implementation of a hashMap good practice? The values will never change and the values used will be needed within the super class and its sub classes (Hence the "protected"). Can I set the key and value variables to final as well?

3) I understand that HashMap doesn't allow for duplicate keys, but from tinkering around with the HashMap, by inputting two of the same keys it simply replaces the older key and value with the newest .put("name", value);Is there to a way to throw an error perhaps if you try to .put("water", 0xff00ff21) and .put("water", 0xff221133) and/or .put("water",0xff00ff21)

Thank you for your time. New to this community and looking forward to helping/getting helped.

4
  • 1
    To prevent replacing a value, call containsKey() first. --- It appears that keys are predefined String literals (e.g. "water"). If that is true, then you should consider using an enum instead of String literals. There is an improved version of Map, keyed by enum, called EnumMap.
    – Andreas
    Jan 19, 2016 at 3:59
  • Well, what happens when and if it's updated? Would it be possible to observe it and get notified of updates? Then you don't have to check. Jan 19, 2016 at 4:05
  • Making map static final means only the reference to the map is final and map will get initialized only once. But it does not guarantee that the elements inserted in map are final. Jan 19, 2016 at 4:08
  • @Andreas call containsKey() first is probably the worse approach if it is going to be run in multi-thread environment. Jan 19, 2016 at 6:01

3 Answers 3

0

Please note that it is bad to ask three questions in one post.

1) IMO, yes. I usually use a HashMap for this kind of things. This can clarify things a lot better and enhance the readability of your code. Just imagine if you you only use hex color values for these kinda things, I think a lot of people would ask you what is 0xff221133 and what is 0xff00ff21.

2) Yes it is! static final is used when you want to declare some kind of constant. However, declaring a hash map as static final doesn't mean that its content cannot be changed. To prevent this, encapsulate the map in a class and only provide get methods:

final class TileColorMap {
    private static final HashMap<String, Integer> tileColorMap = //blah blah blah
    static {
        //Add things to your map here
    }
    public static int get(String key) {
        return tileColorMap.get(key);
    }
}

3) If you look at the docs, specifically, Hashmap<>.put, you will see that:

Returns: the previous value associated with key, or null if there was no mapping for key. (A null return can also indicate that the map previously associated null with key.)

So you can add a method that put something into the map and will throw an exception if the key is a duplicate by checking whether the returned value is null.

private static void putStuffInMap (String key, int value) {
    Integer returnedValue = tileColorMap.put(key, value);
    if (returnedValue != null) {
        throw new RuntimeException("Duplicate Keys!");
    }
}
1
  • My apologies about the the three question post. Just wanted to get similair questions out of the way. And thank you for the great answer!
    – Logiquol
    Jan 19, 2016 at 17:05
0

1) I'm not sure I understand what you're doing here, but how many different kinds of tiles could you be using here? You might be better off just defining a Tile object with a few constant Tiles that you can just reuse again and again by referring to Tile.WATER, etc instead of doing a hashtable lookup. If water has multiple colors just put them all in the water Tile object and pick from amongst them.

public class Tile
{
    public static final Tile WATER = new Tile(...);
    public static final Tile ROCK = new Tile(...);
}

2) Making a hashmap instance static and final doesn't make it immutable. The contents can still be updated. There's no performance benefit anyway. A read only hashmap wouldn't be any faster than a writable one. If you don't want it updated, just don't update it. It's your code, it's not like it's going to write to the hashmap when you aren't looking.

3) You could subclass hashmap and make it not accept duplicate keys, but again, I'm not sure what the purpose of this is- why aren't you sure what colors your tiles will be at run time? This strikes me as the kind of thing decided before compile time.

1
  • Yes, I am using a lot of tiles. The tiles class I have implemented gives a attributes to a specific "Tile" and my world is quite big. The reason for not allowing duplicates is that in the past I've accidentally added an identical 0x number and as you can imagine catching the error was a bit difficult as I had also added about 10 other tiles. Thanks alot for the feedback.
    – Logiquol
    Jan 19, 2016 at 17:04
0
  1. Using HashMap should be efficient enough. Is there more efficient way? Of course there will always be but whether it is appropriate depends on your design. For example, if tiles are statically defined, you may use enum/integer constants to represent a tile (instead of using "name"), and your tile-to-XXX mapping can be easily expressed as ArrayList or even array. (Again, it may not be appropriate to your design).

  2. Again it depends on the design. Are the class containing the map something that is going to instantiate multiple times but you really want each instance to share same mapping? Are you going to give flexibility to the child class to set up the mapping? It is only meaningful to make it static if first answer is YES and second is NO.

    To avoid change of content for the map, you can wrap it in a unmodifiable map:

    // Access your data through this, so you won't mistakenly modify it
    protected final Map<...> tileColorMap = Collections.unmodifiableMap(getTileColorMap());
    
    // your super class or sub-class is providing the actual map
    protected Map<...> getTileColorMap() {
        Map<...> tileColorMap = new HashMap<>();
        // do your setup
        return tileColorMap;
    }
    
  3. If you are using Java 8+, it may be better to use Map#merge() method, and have the remapping function throw an exception you desire. Compared with the approach given by other answers, using merge() is safer as original value won't be mistakenly replaced. You may also selectively throw the exception if the new value is different from existing value.

1
  • Okay, thanks for the info. I wish I could up vote all of you ;p. Dam rookie status lol.
    – Logiquol
    Jan 19, 2016 at 17:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.