Am I storing this HashMap right?

Discussion in 'Plugin Development' started by MooshViolet, Dec 14, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    MooshViolet

    Hi i was just wondering if I was using hashmaps correctly. I want when a player runs a command, it saves the location in a hashmap.
    When they run another command, it brings them back to that location.
    Here is my hashmap code:
    Code:
    Map<String, Location> locations = new HashMap<String, Location>();
    this is when a player runs the first command to store their location:
    Code:
                    locations.put(p.getName(), p.getLocation());
    this is when the player runs the second command to return themself:
    Code:
                Location loc = locations.get(p.getName());
    I was just wondering if this would work.
    Thanks
     
  2. Offline

    Rocoty

  3. Offline

    Dragonphase

    Rocoty likes this.
  4. Offline

    MooshViolet

    @Dragonphase @Rocoty
    Ok I will try it later.
    Thats good news that you say that, so hopefully there will be nothing wrong with it:)
     
  5. Offline

    Jayke_

    Looks good. You could just store the Player object itself as the key value, but a name works too.
     
  6. Offline

    Dragonphase

    @Jayke_

    No. What happens when the player leaves the server? You're left with entries within your map with key references to null Player objects.
     
  7. Offline

    Rocoty

    @Dragonphase PlayerQuitEvent...just remove the player from the hashmap. Honestly, storing the Player object is a whole lot more efficient than having to lookup the player with the UUID every time you need it.
     
    aaomidi likes this.
  8. Offline

    Jayke_

    @Dragonphase Well you wouldn't need to retrieve the values after that, so it wouldn't pose as an issue.
     
  9. Offline

    Feindbild

    No, you should not use the player object as a key, this will create a huge overhead!
    Use UUIDs instead of names, since names will soon stop being unique/static.
     
    ProMCKingz, caelum19 and Dragonphase like this.
  10. Offline

    caelum19

    Plus, I'm pretty sure you need to process 4 bytes for every character in a player's name to retrieve it from a set.
     
  11. Offline

    Rocoty

    I've explained why it won't. Now please provide me with an explanation as to why it will.
     
    aaomidi likes this.
  12. Offline

    aaomidi

    Thats why you make WeakHashMaps

    Stop talking, just stop. Learn what hashes are and how hashmaps work.
     
    Rocoty likes this.
  13. Offline

    fireblast709

    If you can't spare a byte or two for a factor n improvement on your worst case running time*, feel free to use UUIDs.

    *Assume the following snippets:
    Code:java
    1. Map<UUID, V> uuidMap = new HashMap<UUID, V>();
    2. public void foo()
    3. {
    4. for(Map.Entry<UUID, V> entry : uuidMap.entrySet())
    5. {
    6. Player player = Bukkit.getPlayer(entry.getKey());
    7. if(player != null)
    8. {
    9. bar(player, entry.getValue());
    10. }
    11. }
    12. }

    Code:java
    1. // WeakHashMaps are technically not needed with the EventHandler
    2. // Though people add just in case Bukkit would screw up
    3. Map<Player, V> playerMap = new WeakHashMap<Player, V>();
    4. public void foo()
    5. {
    6. for(Map.Entry<Player, V> entry : playerMap.entrySet())
    7. {
    8. bar(entry.getKey(), entry.getValue());
    9. }
    10. }
    11.  
    12. // This avoids your map leaking memory if other plugins
    13. // mess up and leak a strong reference
    14. @EventHandler
    15. public void on(PlayerQuitEvent event)
    16. {
    17. playerMap.remove(event.getPlayer());
    18. }
    Now, for the fun part. Bukkit.getPlayer(UUID) loops over all players, which means it runs in O(n). Your for loop runs n iterations, so that's n * O(n) = O(n^2). The second snippet, on the other hand, runs in n iterations with a simple Map#get(Player), which is said to run in O(1). n * O(1) = O(n), which is n times faster than your code.
     
    Dragonphase, aaomidi and Feindbild like this.
  14. Offline

    Feindbild

  15. Offline

    Monkey_Swag

    And I'm just sitting here going like 'pft, it's much easier to use YAML to save all this'.
     
  16. Offline

    SuperOriginal

  17. Offline

    aaomidi

    -_- ?!?!?! -_-
     
    zackpollard likes this.
  18. Offline

    zackpollard

    You need to learn the difference between store and save. If your post wasn't leaking sarcasm then you would actually be correct, because HashMap's wouldn't save anything.
    Store vs Save guys, gawddd

    - Zack
     
    caelum19, Rocoty and aaomidi like this.
  19. Only use UUID's if you need to store them somewhere for a long time, or it's something crucial like bans or something like that. Also do what aaomidi and fireblast said, remove players when they log out and use weak hashmaps.
     
Thread Status:
Not open for further replies.

Share This Page