`Account`: Change the return type of `retrievePermissionsMap`

[lokka30]

Issue

The return type of Account#retrievePermissionsMap is annoying, and in my opinion, unwarranted:

CompletableFuture<Response<Map<UUID, Set<Map.Entry<AccountPermission, TriState>>>>>

Potential Solutions

I'll update this post over time with some potential solutions:

• Nested Map Not only is this method easier to implement, it is more importantly easier for plugins to use, since they can quickly get(...) the account member's permission state map and then just as quickly get(...) the account permission they want to find the state of.

  CompletableFuture<Response<Map<UUID, Map<AccountPermission, TriState>>>>

• PermissionHolder Object

  // Option 1
  CompletableFuture<Response<Set<EconomyPermissionHolder>>>

  // Option 2
  CompletableFuture<Response<Map<UUID, EconomyPermissionHolder>>>

  This would rename the method from retrievePermissionsMap to retrievePermissionHolders.

  Instead of returning a nested map (or any Map for that matter), a set of permission holder objects is returned.

  The permission holder object would contain the UUID of its associated account member, and an internal map of permission states.

  This is the easiest solution for users, and it's likely easier to implement than the current return type on the economy provider side of things.

Current Opinions

• @lokka30

  I'm equally happy with using a nested map or permission holders, as either way, people no longer need to deal with the current mess. Before I change my opinion to favour a certain solution, I'd like to hear what other people think.

• @MrIvanPlays

  Unknown - it seems he likes permission holders and strongly dislikes nested maps.

• @yannicklamprecht

  Prefers using the PermissionHolder system, "option 2", a map of UUIDs against corresponding permission holder objects.

---

I would appreciate to hear the opinions of @MrIvanPlays, @yannicklamprecht, and anyone else who is interested in partaking in this discussion.

[yannicklamprecht]

Would rather do that like the following.

CompletableFuture<Response<Map<UUID, PermissionHolder>>> retrievePermissionMap()

[lokka30]

Would rather do that like the following.

CompletableFuture<Response<Map<UUID, PermissionHolder>>> retrievePermissionMap()

I see the merit in keeping a UUID map present, if we go for the permission holder solution then I would also prefer this option over eliminating both maps.

[lokka30]

Thank you for your response by the way. :)

[MrIvanPlays]

PermissionHolder has the potential of creating much bigger overhead rather than what is currently implemented. I don't understand what's your problem with the return type?

[lokka30]

PermissionHolder has the potential of creating much bigger overhead rather than what is currently implemented. I don't understand what's your problem with the return type?

I can live with the unusual and verbose return type remaining (nesting a set of map entries inside a different map), though I'd like to see if it can be improved in any form before we make the choice final.

You're right about the overhead concern. What do you think about nesting the map instead of taking the weird side-road solution of using the set of map entries?

[MrIvanPlays]

the current solution allows for caching entries. each time you call map#entrySet (afaik) the set of entries is always generated, hence greater overload. the current solution is good in many ways, as it is much easier to loop through it rather looping through a nested map.

[lokka30]

I greatly appreciate you providing your opinion on this matter, I'm still unsure myself which way to go, though as you insist that the current solution is adequate, I'll close this issue and we'll leave it as-is 🙂