C++: std::map and std::set aren't ordered if using custom class (not pointers)

Question

This must be something incredibly stupid, yet I can't manage to make head or tail from it. This is the testing code.

#include <iostream>
#include <vector>
#include <limits>
#include <random>
#include <map>
#include <set>
#include <stdlib.h>
class value_randomized
{
public:
    double value;
    long random;
    value_randomized()
    {
        value=0;
        random=0;
    }
    /*
    bool operator<(const value_randomized& b) const
    {
        if (value<b.value) return true;
        return (random<b.random);
    }
    */
    friend bool operator<(const value_randomized& a, const value_randomized& b);
};
inline bool operator<(const value_randomized& a, const value_randomized& b)
{
    return (a.value<b.value)?true:(a.random<b.random);
}

int main(int argc, char *argv[])
{
    std::map<value_randomized,size_t> results;
    for (size_t i=0; i<1000; ++i)
    {
        value_randomized r;
        r.value=rand();
        r.value/=RAND_MAX;
        r.random=rand();
        results.insert(std::make_pair(r, i));
    }
    std::multiset<value_randomized> s;
    for (size_t i=0; i<1000; ++i)
    {
        value_randomized r;
        r.value=rand();
        r.value/=RAND_MAX;
        r.random=rand();
        s.insert(r);
    }
    return 0;
}

I've tried overloading the operator< both within the class and outside the class. I've tried both maps and (multi)set. Yet I can't understand how the results are ordered. I'll show the screens of the debug window

Debug screen1

Debug screen2

Even printing directly the value of the multiset gives as result something apparently unordered. These are the first results

(0.0515083,114723506)
(0.0995593,822262754)
(0.0491625,964445884)
(0.410788,11614769)
(0.107848,1389867269)
(0.15123,155789224)
(0.293678,246247255)
(0.331386,195740084)
(0.138238,774044599)
(0.178208,476667372)
(0.162757,588219756)
(0.244327,700108581)
(0.329642,407487131)
(0.363598,619054081)
(0.111276,776532036)
(0.180421,1469262009)
(0.121143,1472713773)
(0.188201,937370163)
(0.210883,1017679567)
(0.301763,1411154259)
(0.394327,1414829150)
(0.383832,1662739668)
(0.260497,1884167637)

Clearly I'm missing something, but what?

Your comparer is wrong (Broken [weak ordering](https://en.wikipedia.org/wiki/Weak_ordering)). Use `return std::tie(a.value, a.random) < std::tie(b.value, b.random);` instead. — Jarod42, Oct 27 '21 at 13:52
@Jarod42 It works.Thanks. But why comparing it field by field didn't work? Edit: Oh, I see the link. I'm going to read it. Thanks again. — DrHell, Oct 27 '21 at 13:59
... it's wrong because it would treat something like `(0.6, 1)` as less than `(0.5, 2)`. That's not how lexicographic ordering should be implemented. — StoryTeller - Unslander Monica, Oct 27 '21 at 14:00

score 4 · Accepted Answer · answered Oct 27 '21 at 14:03

Your comparison function does not do strict weak ordering in accordance with the Compare requirement.

Fixing the existing code could be done like this:

inline bool operator<(const value_randomized& a, const value_randomized& b) {
    if(a.value < b.value) return true;
    if(b.value < a.value) return false;
    return a.random < b.random;
}

or simpler, use std::tie:

inline bool operator<(const value_randomized& a, const value_randomized& b) {
    return std::tie(a.value, a.random) < std::tie(b.value, b.random);
}

C++: std::map and std::set aren't ordered if using custom class (not pointers)

1 Answers1