4

Is it OK to use a NOT on (negate) a Set.contains() method, or should I use an if/else, or does it not really matter?

Use NOT on contains():

if (!psxAssigneeIds.contains(cgm.MemberId)) {
    cgUsersToDeleteSet.add(cgm);
}

or is it better to use else:

if (psxAssigneeIds.contains(cgm.MemberId)) {
    // do nothing
} else {
    cgUsersToDeleteSet.add(cgm);
}

or would both work equally well?

2 Answers 2

8

It is perfectly fine to negate the contains result.

Boolean isValueNotInSet = !someSet.contains(someValue);

Notice, however, that it is not necessary to check for yourself if the Set already contains a value when adding to it. Straight from the documentation:

add(setElement)
Adds an element to the set if it is not already present.

2
  • And if you want to be really pedantic just do if (someSet.contains(someValue) == false) Commented Feb 29, 2016 at 0:32
  • 5
    Yeah...I am not a fan of comparing Boolean values to each other instead of just evaluating them, but that's just me. @BorisBachovski
    – Adrian Larson
    Commented Feb 29, 2016 at 0:33
7

Don't do either: when adding to a set, the values are automatically deduplicated.

// Adds the value if not already present in the set.
cgUsersToDeleteSet.add(cgm);

Also, there's a convenient method called removeAll that lets you avoid doing the initial IF statements. You could use it like this:

cgUsersToDeleteSet.addAll(memberIds);
cgUsersToDeleteSet.removeAll(assigneeIds);
10
  • When adding, it is true that you don't need to check. There are definitely contexts where you do want to though!
    – Adrian Larson
    Commented Feb 29, 2016 at 0:35
  • @AdrianLarson For a map, there's times where you need to check, but for a set, there's never a situation where you need to check for an existing value first (not that I've seen; I'd love to see a counterexample).
    – sfdcfox
    Commented Feb 29, 2016 at 0:37
  • Not during the adding process, I mean there are valid use cases for Set.contains. Whitelisting, for example.
    – Adrian Larson
    Commented Feb 29, 2016 at 0:37
  • In this case, I've got a set of assigneeIds from a permission set, and a set of chatter group members from a chatter group. If the cgmember memberID is not in the set of permission set assignee ids, then I need to do something. Something being add the chattergroupmember to a list of records to delete. So I'm not checking if the record is already in the set I'm adding to, but I'm comparing the two and then taking action, on the negative scenario. Prob many better ways but that's what v2 is for!
    – gorav
    Commented Feb 29, 2016 at 0:46
  • 2
    @gorav I just realized you were using two sets. As an alternative, you might also look at allAll/removeAll to avoid using IF statements unnecessarily.
    – sfdcfox
    Commented Feb 29, 2016 at 0:52

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .