7

I have the following List of objects:

private List<Object> teamlist = new ArrayList<Object>();

And I'm adding objects to the list like so:

teamlist.add(new MCWarTeam(args[0], joinkey));

Now the objects in the list have no name, but can be referenced by using the list, right? Before I add a new element to the list, how can I check if an object with a certain attribute already exists? This is the constructor of the Objects:

public MCWarTeam(String teamname, String joinkey){
    this.teamname = teamname;
    this.joinkey = joinkey;
}

I want to check if there already is a team with the name teamname. Alternatively, is there a better way to store the Objects? Before, I just used a HashMap to add the teamname and joinkey and it worked just fine, but figured using Objects instead would be a better way to do it.

Here is the important code for the event handler:

        else if (cmd.getName().equalsIgnoreCase("createTeam")) {
        if (args.length > 0 && args.length < 3) {
            String joinkey = "";
            if (args.length > 1)
                joinkey = args[1];

            String teamname = args[0];

            MCWarTeam newTeam = new MCWarTeam(teamname, joinkey);
            if (!teamlist.containsKey(teamname)) {
                teamlist.put(teamname, newTeam);
                sender.sendMessage("Created new team \"" + teamname + "\" with join key \"" + joinkey + "\" successfully! Teams:");

                sender.sendMessage("All teams:");
                for (String key : teamlist.keySet()) {
                    sender.sendMessage(key);
                }

            } else
                sender.sendMessage("Team already exists!");
            return true;
        }
        return false;
    }

    else if (cmd.getName().equalsIgnoreCase("joinTeam")) {
        if (args.length > 0 && args.length < 3) {
            String joinkey = "";
            if (args.length > 1)
                joinkey = args[1];

            String teamname = args[0];

            if (teamlist.containsKey(teamname)) {
                String teamKey = teamlist.get(teamname).getJoinKey();
                if (joinkey == teamKey) {
                    teamlist.get(teamname).addPlayer(playername);
                    Bukkit.broadcastMessage("MCWar: " + playername + " joined Team \"" + teamname + "\" successfully!");
                } else
                    sender.sendMessage("Join key incorrect!");
            } else {
                sender.sendMessage("Team doesn't exist! Teams:");
                for (String key : teamlist.keySet()) {
                    sender.sendMessage(key);
                }

            }
            return true;
        }
        return false;
    }

Basically, if it returns false, the user will get a message explaining the correct usage of the command he entered.

Mick Mnemonic
  • 7,808
  • 2
  • 26
  • 30
Martin
  • 453
  • 3
  • 5
  • 14
  • Do you override `equals` in MCWarTeam? If so, just use `List.contains`. – Andy Turner Oct 25 '15 at 11:46
  • You probably want to type the list to `` instead of `` as well. Also I don't know how you figured that an `ArrayList` is somehow inherently better than a `HashMap`, but I hope you based that on something real and not just a gut feeling. – Kayaman Oct 25 '15 at 11:48
  • Is it that hard to read the [specification](http://docs.oracle.com/javase/7/docs/api/java/util/ArrayList.html)? – Hot Licks Oct 25 '15 at 11:49

4 Answers4

8

Java's List<T> has a boolean contains(Object) method, which is handy for situations when you wish to avoid duplicates:

if (!teamlist.contains(newTeam)) {
    teamlist.add(newTeam);
} 

MCWarTeam class must implement equals in order for this to work. When you override equals, you must also override hashCode.

@Override
public boolean equals(Object obj) {
    if (!(obj instanceof MCWarTeam))  {
        return false;
    }
    MCWarTeam other = (MCWarTeam)obj;
    return teamname.equals(other.teamname)
        && joinkey.equals(other.joinkey);
}
@Override
public int hashCode() {
    return 31*teamname.hashCode()+joinkey.hashCode();
}

I'm just looking to check if an Object with the same teamname already exists, but not care about the joinkey?

If joinkey is not part of your object's state that influences equality, it is usually not a good idea to keep it as part of the object as a field. For example, if joinkey is something transient which you use to "connect" teams to other things, making a HashMap<String,MCWarTeam>, using joinkey as the key to the map, and removing joinkey from MCWarTeam should be a good idea.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • joinkey is something like a password the user will have to enter to become part of a team but I see how the name might be misleading. Can you explain to me how `MCWarTeam other = (MCWarTeam)obj; return teamname.equals(other.teamname);` works to see if there already is an object with the same name in the List? – Martin Oct 25 '15 at 12:49
  • 1
    @MartinHoffmann `teamlist.contains(newTeam)` method calls `equals(Object)` method under the hood, comparing `newTeam` to what's already on the list. – Sergey Kalinichenko Oct 25 '15 at 12:52
  • how can I get the String `joinkey` from a `teamname`? I tried the following: `String teamKey = teamlist.get(teamlist.indexOf(teamname)).getJoinKey();` and tried using a test MCWar-object with the same name in place of `teamname` in `indexOf()`, but that didn't work either, do I have to @Overwrite `indexOf()` as well? – Martin Oct 25 '15 at 13:24
4

Based on the description and your comments to other answers, it seems like a good idea to not use a List, but instead store your data in a Map<String, MCWarTeam>, which maps team names into MCWarTeam objects:

private Map<String, MCWarTeam> teams = new HashMap<>();

You can add a team, checking whether a team with the same name already exists, like this:

String teamName = args[0];

if (!teams.containsKey(teamName)) {
    teams.put(teamName, new MCWarTeam(teamName, joinKey));
} else {
    // do what you want when the team name was already in the map
}

Retrieving an MCWarTeam object based on team name, e.g. for accessing the joinKey attribute, is easy:

String joinKey = teams.get(teamName).getJoinKey();

Note that using this approach, you shouldn't implement equals or hashCode in MCWarTeam, because you aren't gonna need it; as your map keys are team names, containsKey operates on String objects which already have well-defined equals and hashCode semantics.

Mick Mnemonic
  • 7,808
  • 2
  • 26
  • 30
  • This really seems like the best way I've seen to do this. I'll try it when I'm at home, thanks – Martin Oct 25 '15 at 16:59
  • I'm getting a weird error here. In a function, I add teams with `teamlist.put(teamname, newTeam);`. Now every time I check if an entry exists, via `teamlist.containsKey(teamname)`, it appears empty. No results show up when I let it run through a loop, but when I run the exact same loop after adding an element, all the added elements show up. EDIT: this is when I call /createteam and /jointeam after each other http://prntscr.com/8vpdnk – Martin Oct 26 '15 at 22:39
  • It's hard to say what might be happening there without seeing the code. Could you edit your question and post the code where you add teams / check for entries? – Mick Mnemonic Oct 26 '15 at 23:47
  • I did, thanks. The HashMap teamlist is defined above. – Martin Oct 27 '15 at 00:12
  • One thing I noticed is that you're comparing the join key `String`s with `==`; you should use `equals` instead. You could try changing the comparison to `joinkey.equals(teamKey)` and see if it has any effect. Other than that, it's still hard to say what your actual problem is. I'm not familiar with Bukkit or its event handling framework, but there might also be some timing issues playing a part here if the events are handled asyncronously. Posting more code will help tracking the problem down. Also, please use braces (`{ }`) in all `if`s and `else`s (even ones containing a single statement). – Mick Mnemonic Oct 27 '15 at 00:49
  • I happen to be in Uni right now but unfortunately the last version of the code I had saved is from 2 days or so ago. But I did try to reconstruct as much as possible as well as make changes to your code like you suggested. In fact I didn't even know why I compared these two strings with ==, I already learned that .equals is a better way to do it. Now I'm almost certain my problem is based on the way Bukkit is built. I can use the same loop to list all teams in the list every time after I create a new one, then have none show up when I use it in `jointeam` and they http://pastebin.com/YZzaYyk9 – Martin Oct 27 '15 at 09:23
2

If you implement MCWarTeam equals method properly, then contains should tell you if the object exists.

 boolean exists = teamlist.contains(member);

And as @Eran mentioned a HashSet would give you O(1) lookup where list contains is O(n), the only thing is that HashSet doesn't allow duplicates. And Yes, use the actual type rather than Object

List<MCWarTeam> teamlist = new ArrayList<>();
Sleiman Jneidi
  • 22,907
  • 14
  • 56
  • 77
  • A `Set` may be impractical for the OP because even though `contains` performs well, the only way to _retrieve_ elements is through iteration; there is no `get` method. A `Map` might be the best alternative for this reason. – Mick Mnemonic Oct 25 '15 at 12:42
2

In order to search for an MCWarTeam instance in the ArrayList, you'll first have to override equals in order to define what it means for two MCWarTeam instances to be equal to each other. Then you can use indexOf(team) or contains to determine whether a instance is in the List.

However, such a search would take linear time, so a HashSet may be better for your needs (for that purpose you'll need to override both equals and hashCode, and you'll be able to find if an object is in the Set in constant time).

Eran
  • 387,369
  • 54
  • 702
  • 768