Multiple if in comparison function vs chain of || or &&

Question

I'm writing a function (or operator==() in C++), comparing two objects. The code looks like this:

bool operator==(const Obj& copy1, const Obj& copy2)
{
    if(copy1.a!=copy2.a)
        return false;
    if(copy1.b!=copy2.b)
        return false;
    if(copy1.c!=copy2.c)
        return false;
    return true;
}

There may be many more data members to compare, and not all members present in Obj are to be compared, so using memcmp() isn't an option. There also might be more complicated tests than just equality of the members. Now there's another way:

bool operator==(const Obj& copy1, const Obj& copy2)
{
    return (copy1.a==copy2.a &&
            copy1.b==copy2.b &&
            copy1.c==copy2.c);
}

The first way seems more verbose, but also easier to modify, while the second seems somewhat fragile (can't explain why, just gut feeling), but looks a lot simpler.

What is a better way? Is there any real difference in readability or maintainability of these approaches?

Use the second, and see if you can't get rid of the more complex conditions by wrapping the relevant data in a separate class that itself overloads `==` — Sebastian Redl, May 28 '15 at 13:18

score 1 · Accepted Answer · answered May 28 '15 at 13:14

1

The first example is bad/fragile due to a lack of braces. Such code would not pass code review at the majority of places I've worked. If you add braces (making the conditional a single line if preferred), then they're fairly equivalent from what I've seen. Many places will prefer the first since conditionals can be used everywhere, but the complex expression can only be used when its parts are relatively simple/similar. But they wouldn't object to the second version in this sort of case, and it won't lead to maintenance problems.

answered May 28 '15 at 13:14

Telastyn

108,850
29
239
365

+1 One could either omit the braces, or write it in one line. But the form above is double inconsequent. We all remember Apple's _Goto-Fail_. – Thomas Junk May 28 '15 at 13:16
By braces do you mean `{}` or parentheses `()`? I don't quite understand, lack of what you are talking about. Do you mean I should write `{return false;}` instead, or `(copy1.a==copy2.a)&&...`? – Ruslan May 28 '15 at 13:33
@ruslan - `if(blah){stuff;}` rather than `if(blah)stuff;`. http://programmers.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no – Telastyn May 28 '15 at 14:03

Multiple if in comparison function vs chain of || or &&

1 Answers1