1

Similarly to Preferred implementation of '<' for multi-variable structures I'm implementing a less-than operator for a structure with multiple values. I'm not worrying about using equality or less than operators, lets assume all the members correctly implement both. My structure has four fields and the operator is already getting quite messy:

struct Key {
    std::string s_name;
    bool b_mipmaps;
    bool b_clamp_to_edge;
    GLenum n_intformat;

    // [constructors here]

    bool operator <(const Key &other) const
    {
        return s_name < other.s_name || (s_name == other.s_name && (
            b_mipmaps < other.b_mipmaps || (b_mipmaps == other.b_mipmaps && (
            b_clamp_to_edge < other.b_clamp_to_edge || (b_clamp_to_edge == other.b_clamp_to_edge && (
            n_intformat < other.n_intformat))))));
        // compare two keys
    }
}

I was wondering if there is a commonly used style of indentation or something that helps you not to get lost in the parentheses because frankly, it is hell and I imagine a bug in such an operator would be quite subtle and hard to track down / debug. Is there perhaps a way to break this down to some primitive functions? Or is there an STL function that does this?

I'm currently using C++03 but I'm open-minded about newer standards.

Community
  • 1
  • 1
the swine
  • 10,713
  • 7
  • 58
  • 100
  • 1
    Is it absolutely required to use a single statement for this? – Jongware May 22 '15 at 15:59
  • 1
    Break it to multiple statements and variables? – simon May 22 '15 at 15:59
  • @Jongware How do you use multiple statements? Is there an obvious advantage? You'll just get lost in curly braces instead? – the swine May 22 '15 at 16:00
  • This is not a duplicate- the name of the other question assumes knowledge of `tie` and `tuple`, not to mention potential issues with C++11 compatibility. – Puppy May 22 '15 at 16:00
  • @Puppy The other question provides good answers for implementing `operator<` with multiple members even if the titles aren't the same so I thought it was a good duplicate candidate. – Mark B May 22 '15 at 16:05
  • @MarkB what question are we talking about in here? Couldn't find it in search. Could be helpful for me, although I use C++03 for the time being. – the swine May 22 '15 at 16:06
  • @the swine http://stackoverflow.com/questions/6218812/implementing-comparison-operators-via-tuple-and-tie-a-good-idea – Mark B May 22 '15 at 16:07

4 Answers4

2

You can use several if's.

bool operator <(const Key &other) const
{
    if (s_name != other.s_name) return s_name < other.s_name;
    if (!b_mipmaps && other.b_mipmaps) return true;
    if (b_mipmaps && !other.b_mipmaps) return false;
    if (!b_clamp_to_edge && other.b_clamp_to_edge) return true;
    if (b_clamp_to_edge && !other.b_clamp_to_edge) return false;
    return n_intformat < other.n_intformat;
    // compare two keys
}
timrau
  • 22,578
  • 4
  • 51
  • 64
  • Right, that's almost a pattern. If you did with the bools the same as with strings, it would be quite clean. A reasonably optimizing compiler should be able to get to the same result as my single statement. – the swine May 22 '15 at 16:02
2

Not knowing what the types of the variables in your code are, I find it hard to suggest something using your variable names.

From your code, it's not clear what the semantics of the operator should be if

(this->b_mipmaps && other.b_mipmaps) is true.

I use the following pattern:

bool operator <(const Key &other) const
{
   if ( this->member1 != other.member1 )
   {
      return (this->member1 < other.member1);
   }

   if ( this->member2 != other.member2 )
   {
      return (this->member2 < other.member2);
   }

   //
   // ...
   //

   return (this->memberN < other.memberN);
}

EDIT

Now that I know that b_mipmaps is of type bool, you can use:

   if ( this->b_mipmaps != other.b_mipmaps )
   {
      return (this->b_mipmaps < other.b_mipmaps);
   }

or

   if ( this->b_mipmaps != other.b_mipmaps )
   {
      return (!(this->b_mipmaps) && other.b_mipmaps);
   }

Go with the style that you find more readable. I find the first form more readable.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 1
    Yes, that's the pattern from timrau's answer, with more curly braces which are not really needed. Nice, though, didn't think of that. I guess I've been using too much Prolog :). – the swine May 22 '15 at 16:08
  • 1
    @theswine, Those additional braces, even though they are not needed in this case, have saved me from quite a few bugs :) – R Sahu May 22 '15 at 16:16
  • @theswine: this is what I had in mind (never minding the "curly braces" bit). Each line is an early opt-out in itself, and, if taken, will return the correct result. – Jongware May 22 '15 at 16:17
  • Oh, sorry for not posting the declaration of my structure. `s_` is `std::string`, `b_` is `bool` and `n_` is any `int`-ish type. I know that i can just less-than compare bools but it seems like a jinx to me for whatever reason. – the swine May 22 '15 at 16:19
  • If the corresponding boolean members in both structures are true, then they are equal, and the test is moving on to the next variable. Correct me if I'm wrong but i believe for bools, `!b_mipmaps && other.b_mipmaps` is equivalent to `b_mipmaps < other.b_mipmaps`, no magic here. Isn't it? – the swine May 22 '15 at 16:24
  • @theswine, those are equivalent. Go with the style that you find more readable. – R Sahu May 22 '15 at 16:28
2

You can use std::tie for that:

bool operator <(const Key &other) const
{
    return std::tie(s_name, b_mipmaps, b_clamp_to_edge, n_intformat) <
        std::tie(other.s_name, other.b_mipmaps, other.b_clamp_to_edge, other.n_intformat);
}

Do not forget to #include <tuple>

lisyarus
  • 15,025
  • 3
  • 43
  • 68
  • That's certainly very clean, I'm using C++03 though, and I'm not going to bring Boost just for this. I'll keep that in mind for when I switch to C++14. – the swine May 22 '15 at 16:11
  • @theswine C++11 is sufficient for `std::tie` :) Maybe worth including information about C++ standard version in the question? – lisyarus May 22 '15 at 16:14
  • Yes, I'm aware of 11. I expect to use 14 on the next big project, skipping 11. I've put a mention of the standard there. – the swine May 22 '15 at 16:28
2

As for @lisyarus excellent answer, but avoiding duplication of the field names:

bool operator <(const Key &other) const
{
    auto fields = [](decltype(*this) v) {
        return std::tie(v.s_name, v.b_mipmaps, v.b_clamp_to_edge, v.n_intformat);
    };
    return (fields(*this) < fields(rhs));
}
Pete
  • 4,784
  • 26
  • 33