Should operators == or != throw in this case?

Question

I'm writing a program where I have created a std::vector of POD structs. One of the members of the struct is a unique identifier.

In order to be able to use std::binary_search I have to implement operator< for the struct. Following the guidelines here, I'm writing the full set of overloads for ==, !=, <, >, >= and <=.

This presents one issue I wasn't sure how to handle. The vector will be ordered by the unique ID I assign each struct. Two structs are the same if they have the same identifier. However, it occurs to me that a situation could crop up if two structs have the same identifier but different data in the other members.

This should never happen. Would it be appropriate to have the comparison operator check the rest of the fields then and throw an exception if they're different but the ID is the same? What sort of exception would be most appropriate?

I think a `std::set` with the comparison based on the id is better suited for this task. — chris, Dec 07 '12 at 00:50
If you want this condition to be recoverable at runtime, then `throw` would be appropriate. If not, `assert` might be a better choice. — phonetagger, Dec 07 '12 at 00:56
@chris I went with std::vector because I need the memory to be contiguous. I'm not aware that set (or any of the other ordered containers) provides this guarantee. — Kian, Dec 07 '12 at 01:28
The comparison here is a red herring. If no two of these structs should exist sharing a unique identifier, why wait until you compare them to find out? Make the constructor for this type private, and provide a factory function that guarantees it. — GManNickG, Dec 07 '12 at 01:33
@GManNickG It's not an error for two copies of the same struct to exist. What is an error is for two structs to exist that share an ID but different information. And if I did that, the structs would no longer be PODs. — Kian, Dec 07 '12 at 01:38
@Kian: Yes, but you didn't say they needed to be PODs, just that they were. But anyway, I see what you mean now, kind of. If more than one copy exists, it would be impossible to update one without brekaing that contract. Does this happen, or are you saying that it's just possible in principle? — GManNickG, Dec 07 '12 at 01:45
@GManNickG When I push_back a struct I have to first build a dummy. If I make an array for example they'll all have an ID of 0. It is possible that once in the vector I modify the struct, and a copy becomes illegal. But I should never compare them in any of those cases. — Kian, Dec 07 '12 at 02:13

score 7 · Accepted Answer · edited May 23 '17 at 11:55

This is merely an expansion on SCombinator's answer.

The fact that you said "This should never happen." means you want to use an assert, not an exception (or a combination of the two). An exception would hide the error - well, not hide, but you'll be able to catch it and continue. It is better suited for exceptional situations which you didn't really plan for - for example you're attempting to open a file that doesn't exist. It's not really part of the logic that the file is missing, it's your little brother accidentally deleting your file, or an aggressive anti-virus that thought it was a virus, or whatever - it's just an exceptional situation.

If members with the same ID but otherwise different is something that should never happen, that's basically an assertion. It's part of the logic - part of the requirements if you will. Throwing an exception simply points this out, but there's not really a way you can recover from it. By the time you realize something is wrong, it's already 2 late. You have two objects with the same id, yet different, you don't know which one is correct, you don't know why the incorrect one exists, and so on. You probably don't even want to recover from it. The application is already in a erroneous state - the two contradicting objects already exist. Your application is in an un-recovarable state - if you continue with it, you'll probably get wrong results or worse.

If it's not a critical assertion, you can also throw an exception afterwards, and provide a clean way to close the application, but that's just a beautification.

In general, I follow a simple rule - If it's something that can happen under extraordinary circumstances, I use exceptions. If it's something that should never happen, and if it does, means something is seriously wrong with the logic in the code, I use an assert (and possibly a force crash).

Thanks. Just to clarify a point, though, the error occurs if the ID is the same and the members are different, not the other way around. — Kian, Dec 07 '12 at 01:26
@Kian ah, ok. Better now? (it doesn't change the reasoning though) :) — Luchian Grigore, Dec 07 '12 at 01:27
Yes, thanks for the explanation. It does make it more clear when to use each. — Kian, Dec 07 '12 at 01:31

score 2 · Answer 2 · answered Dec 07 '12 at 00:51

2

If such a thing is a logical error, you should probably use an assert.

answered Dec 07 '12 at 00:51

sanjoyd

3,260
2
16
22

An assert is the right thing to use here, but you might want to expand on why, and why it's better than an exception in this case. – Luchian Grigore Dec 07 '12 at 00:56

score 2 · Answer 3 · answered Dec 07 '12 at 00:58

2

Well, you have to ask yourself a few questions:

1) Is this likely to happen during testing (ie signaling a bug) but will never happen during normal execution? If the answer is yes, use an assert.

2) Is this going to happen during normal runtime, and if so, can the program recover and continue execution? If the answer is yes, throw an error, catch and handle it.

3) Is this going to happen during normal runtime and is it unrecoverable? If yes, either call abort or something like that (exit, terminate) or throw an exception which you're not going to handle.

answered Dec 07 '12 at 00:58

Andrei Tita

1,226
10
13

If only 1) were that simple. How can you tell whether some error happens only during testing but never during normal execution? – Luchian Grigore Dec 07 '12 at 01:43
Either hope for the best, or stop using assert altogether, I guess. (I mean use full tests instead of asserts.) It is not always simple. But it is sometimes possible. – Andrei Tita Dec 07 '12 at 01:47

score 1 · Answer 4 · answered Dec 07 '12 at 14:07

Bad data should be rejected when the data is created. It's not the job of comparison operators to weed out mistakes; that should be done before the program does any serious data manipulation. So the answer to the question is no, operator== and operator!= should not throw exceptions on bad data; they should be written with the assumption that the data that's being passed to them is valid.

score 0 · Answer 5 · answered Dec 07 '12 at 00:58

You could just use a map which is implemented with a binary search tree, although you'd be duplicating an id. Then again, you wouldn't even need to store the id in the struct, since they are supposed to be unique.

// Example POD struct.
struct MyStruct
{
  int id; // Redundant.
  char a;     
}

std::map<int, MyStruct> myMap;
MyStruct m; m.id = 1;
myMap[m.id] = m;
// Or simply..
myMap[1] = m;

Should operators == or != throw in this case?

5 Answers5