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?

7

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.

  • And if you want to be really pedantic just do if (someSet.contains(someValue) == false) – Boris Bachovski Feb 29 '16 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 Feb 29 '16 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);
  • When adding, it is true that you don't need to check. There are definitely contexts where you do want to though! – Adrian Larson Feb 29 '16 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 Feb 29 '16 at 0:37
  • Not during the adding process, I mean there are valid use cases for Set.contains. Whitelisting, for example. – Adrian Larson Feb 29 '16 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 Feb 29 '16 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 Feb 29 '16 at 0:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.