public class PlayerManager { private static final Map<String, Player> players = Collections .synchronizedMap(new HashMap<String, Player>()); public synchronized void add(Player player) { players.put(player.getName(), player); } public synchronized void remove(Player player) { players.remove(player); } public List<Player> getPlayers() { return Collections.synchronizedList((List<Player>) players.values()); } public synchronized int getOnlinePlayers() { return players.size(); } public synchronized Player getPlayer(String player) { return players.get(player); } public void sendPacketToAll(PacketWritable packet) { sendPacketToAll(packet, null); } public void sendPacketToAll(PacketWritable packet, Player exceptPlayer) { for (Player player : getPlayers()) { if (player != exceptPlayer) { player.writePacket(packet); } } } public String toString() { StringBuilder output = new StringBuilder(); output.append("[Online:" + getOnlinePlayers() + "]"); boolean first = true; output.append("["); for (Player player : getPlayers()) { if (first) { output.append(player.getName()); first = false; } else { output.append(", " + player.getName()); } } output.append("]"); return output.toString(); } }

When I try to get a list of players using the getPlayers() method, getPlayers() get an error:

java.util.Collections $ SynchronizedCollection cannot be cast to java.util.List.

By googling did not come to a solution. Tell me how to fix it.

  • 2
    Perhaps the problem is in the code (List <Player>) of players.values ​​() Map.values() returns only a Collection . - VladD

3 answers 3

There is no problem, return as an instance of the collection. If you need a list, create a new one and just add all the necessary elements to it. It also seems to me that there is no need to make the methods of this class synchronized. And you can also just use ConcurrentHashMap to store data.

     Collections.unmodifiableList(new ArrayList<Player>(players.values()));

      Your code will be very slow. If you really need synchronized collections, then look towards the concurent package. There are collections for all occasions. And synchronized is a potentially bottleneck code. If you do everything synchronized, then your multithreaded application will lose in speed the same operations performed in one stream.

      If you insist on this solution, then your getPlayers () method is meaningless. Collections.synchronized does not return a regular collection, including a sheet. It returns a synchronized collection, and this is a completely different class. And your synchronized Mar does not return the usual collection. And you are actually trying to take a collection, bring it to the sheet, then make a synchronized sheet, and then return the usual sheet ... Well, you see, nonsense.

      Rewrite the method this way.

       public Collection<Player> getPlayers() { return Collections.synchronizedCollection(players.values()); }

      And if you are not satisfied with the resulting Collection, then after calling the method, use the following code:

       PlayerManager pm = new PlayerManager(); Collection<Player> players = pm.getPlayers(); List<Player> list = Collections.unmodifiableList(new ArrayList<>(players));

      From your method you get a Collection and then from it your sheet.

      But I repeat once again that this is a bad decision ....