1
\$\begingroup\$

The objective

I was trying to create a function that would compare 7 different integers. It should be able to be used to:

  • Sort the two structures in a consistent manner
  • Be able to detect if two structures are equal with !isLessThan(a,b) && !isLessThan(b,a)

The code

At first, I thought of using a hash function to reduce all the values to one, but then it occurred to me that I was over-engineering this simple problem.

This is language agnostic, but I ended up doing it like this:

bool isLessThan(const MyStruct& lhs, const MyStruct& rhs) {
    return
        lhs.a +
        lhs.b * 2 +
        lhs.c * 3 +
        lhs.d * 4 +
        lhs.e * 5 +
        lhs.f * 6 +
        lhs.g * 7
            <
        rhs.a +
        rhs.b * 2 +
        rhs.c * 3 +
        rhs.d * 4 +
        rhs.e * 5 +
        rhs.f * 6 +
        rhs.g * 7;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Your code will wrongfully declare many tuples equal. e.g. a=2, all others 0 vs. b=1, all others 0. \$\endgroup\$ – Snowbody Dec 3 '17 at 7:58
  • \$\begingroup\$ What you have is effectively a hash function. It's just that it's expanded inline, rather than declared as a C++ function. \$\endgroup\$ – Toby Speight Dec 5 '17 at 9:06
3
\$\begingroup\$

Don't re-invent the wheel (unless of course you want practice at doing that):

  1. Instead of naming a bunch of elements that are all the same type, use an array for your structure's elements like std::array, or use a std::tuple. Or if you really don't want to change your structure's content, use std::tie to craft a tuple of your structure's elements on demand.
  2. If you used a regular array use std::lexicographical_compare on the iterable elements of the two structures. A std::array or std::tuple already has non-member function support for operators like <, <=, >=, etc. though.
  3. Kick back and enjoy a brewski. ;) (only if allowed of course and be sure to drink responsibly etc. etc.).
\$\endgroup\$
  • \$\begingroup\$ I actually abstracted away much of my implementation, they are actually a glm::vec4 and a glm::vec3, and well it is a CS assignment to learn OpenGL, so the main purpose is to reinvent the wheel, haha. \$\endgroup\$ – GBF_Gabriel Dec 3 '17 at 15:29
2
\$\begingroup\$

Some things that caught my eye:

You haven't mentioned the specific criteria that would make one struct less than another.

If the variables' values aren't sorted, it is entirely likely to get false positives with your approach.

If g is greater than INT_MAX / 7, or f is greater than INT_MAX /6, etc., or if the total is greater than INT_MAX, your code will throw a fatal error.

\$\endgroup\$
  • \$\begingroup\$ I didn't have any specific criteria for defining what is a bigger struct than other, I just need order. Also, yeah, it can hit INT_MAX easily. Could you elaborate on the false positives part? \$\endgroup\$ – GBF_Gabriel Dec 3 '17 at 2:20
  • 2
    \$\begingroup\$ @GBF_Gabriel - Using an aggregate sum like this, a struct with high values in the early variables might appear less than a struct with lower values in the later variables. Which could be contrary to what's expected. \$\endgroup\$ – tinstaafl Dec 3 '17 at 2:27
1
\$\begingroup\$

there is an easy way to do this:

auot const& left  = std::make_tuple(lhs.a, lhs.b, lhs.c, lhs.d, lhs.e, lhs.f, lhs.g);
auto const& right = std::make_tuple(rhs.a, rhs.b, rhs.c, rhs.d, rhs.e, rhs.f, rhs.g);

return left < right;
\$\endgroup\$

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.