Is it a good practice to make methods "do nothing" based on a condition?

Question

Take a look at this:

public class Main {

    private static List<Integer> list = new ArrayList<>();

    public static void add(int x) {
        if(list.contains(x)) {
            return;
        }
        list.add(x);
    }

    public static void main() {
        list.add(1);
        list.add(1); //should it do nothing or throw an error?
    }

}

Also, please ignore that I could've used a Set<Integer> which removes the need for if(list.contains(x))

Anyways, assume that list cannot have duplicates, and also assume that if at any time a duplicate is accidentally added (ex: the second list.add(1)) that should be considered a bug; I don't want to add duplicates if I don't have to.

The bottom line is this: should add(int x) have instead thrown an exception (like an IllegalArgumentException or something)? I understand that if I didn't, I wouldn't need to worry about it causing actual bugs involving duplication since it just won't do anything on the second add, but it still bothers me a bit that at some point an unnecessary add() could be called.

I have seen code similar to add(int x) that checks something, and just does nothing based on that. Hopefully you can apply this idea to something you have done before.

Anyways, I don't even know. Should I continue like the above in later similar problems, or should it have thrown an exception?

Answer 1

Yes to both, depending on what you want to achieve. For a set, adding a non-unique element should do nothing, and is not an exceptional situation. For a list of unique elements, adding a non-unique element might be an exceptional situation, and should raise an exception. It is not a question of programming, but of modeling: what your object considers normal, and what it does not. There is no one "best" answer.

For example, if you want to track which developers have had any work done today, you might add them to the worked set. If a developer does another commit today, and you add him, he still just "had work done", and it is not an exceptional situation; just return without changing the list.

But if you are handling employment records and employ a developer while he is actually already being in your employ, that is an exceptional situation, and an error should be raised.