How do I Insert std::vector into a std::set which has a custom sort function

Question

Note that the problem here has been solved and it had nothing to do with the insert but rather an uninitialized struct member variable! Hopefully this question and its answer might help another rookie avoid such a mistake.

I want to insert a std::vector of file names into a std::set that has a customer sorter struct for ordering files by date instead of alphabetically.

With the default alphabetical ordering I can simply insert a vector into my set using:

std::set<std::string> mySet;
std::vector<std::string> myVector;

myVector.push_back("Banana.txt");
myVector.push_back("Apple.txt");
myVector.push_back("Cat.txt");

mySet.insert(myVector.begin(), myVector.end());

And this would give me exactly what I would expect: A std::set of file names that would be ordered alphabetically.

Now if I have a custom sorter that sorts by date instead of file name, like so:

struct DateOrderSorter
{
    bool operator()(const std::string& file1, const std::string& file2)
    {
        struct stat buf_stat1;
        struct stat buf_stat2;
        std::string fullpath1 = path + file1;
        std::string fullpath2 = path + file2;
        stat(&fullpath1[0], &buf_stat1);
        stat(&fullpath2[0], &buf_stat2);
        return buf_stat1.st_ctime < buf_stat2.st_ctime;
    }
    std::string path;
};

And I declare my set as:

std::set<std::string, DateOrderSorter>

and then declare an instance of DateOrderSorter:

DateOrderSorter dateOrderSorter;
dateOrderSorter.path = "C:/random_path_that_has_been_verified_to_work"

When I do the same insertion:

mySet.insert(myVector.begin(), myVector.end());

It returns just the first and the last files sorted by date. So just myVector.begin() and myVector.end().

First of all, why this behavior?
What can I do to get the vector inserted into my set ordered through my customer sorter.

I've tried

std::vector<std::string>::iterator vector_it = myVector.begin();
std::vector<std::string>::iterator vector_end = myVector.end();
for (; vector_it!= vector_end; ++vector_it) {
    mySet.insert(*vector_it);
}

But this didn't copy the vector fully and the order was very weird. It didn't follow name or date order..

Yes, I printed out the set and was able to verify that it sorts it in date order — Algorithmic, Jul 04 '18 at 11:12
What happens if multiple files have the same date associated with them? — G.M., Jul 04 '18 at 11:14
My guess is that `stat` returns an error (which is not checked by the code), maybe because `path` is not initialized to a given directory, but default initialized to `""`. — Olivier Sohn, Jul 04 '18 at 11:15
I have ensured that they don't have the same date but my working sorter checks for this and defaults back to alphabetical sort if the dates are the same — Algorithmic, Jul 04 '18 at 11:16
@OlivierSohn jussted, what about the content of `Path` ? does it point to the right Parent Path ? — Blood-HaZaRd, Jul 04 '18 at 11:27
@Algorithmic yes but what you wrote doesn't do what you think it does : use a debugger and see what happens in the comparison function (it's a different instance, and note that `path` is not static). — Olivier Sohn, Jul 04 '18 at 11:32
ok, I see that we're getting a bit fixated on the path issue but consider this to be working as expected. I have verified that it does, I just can't replicate my entire set of code here. — Algorithmic, Jul 04 '18 at 11:45
@Algorithmic I'm not convinced, can you test the return value of `stat`, to verify that the function succeeds (i.e returns zero)? — Olivier Sohn, Jul 04 '18 at 11:49

Olivier Sohn · Accepted Answer · 2018-07-04T20:58:07.877

The path member of the comparison function is default-initialized to "", hence stat errors (the error goes unnoticed because the return value is not checked in the code), and buf_stat1.st_ctime < buf_stat2.st_ctime is comparing uninitialized memory.

Which means that the comparison function can return true for both a < b and a > b : this violates a rule of strict weak ordering, hence you observe this weird behaviour.

To fix this, you can make path a static member, and verify that stat doesn't error, like so :

struct DateOrderSorter
{
    bool operator()(const std::string& file1, const std::string& file2)
    {
        struct stat buf_stat1;
        struct stat buf_stat2;
        std::string fullpath1 = path + file1;
        std::string fullpath2 = path + file2;
        int r = stat(&fullpath1[0], &buf_stat1);
        assert(r==0);
        r = stat(&fullpath2[0], &buf_stat2);
        assert(r==0);
        return buf_stat1.st_ctime < buf_stat2.st_ctime;
    }
    static std::string path;
};

and initialize it like so:

DateOrderSorter::path = "...";

Actually, you were right. It was about that path variable not being initialized. Thanks so much! — Algorithmic, Jul 04 '18 at 11:58
do you mind if I delete this question? It could be quite misleading for someone who's just trying to insert a vector into a set... — Algorithmic, Jul 04 '18 at 12:10
@Algorithmic maybe change the title. Or delete the question, if you want I don't mind. — Olivier Sohn, Jul 04 '18 at 12:12

score 0 · Answer 2 · answered Jul 04 '18 at 11:29

0

Because ctime suggests that it is Posix time, that is of second scale. Of course, two different files might have the same value of this time, that actually means the last time the file's status has changed. Try to use std::mutiset.

answered Jul 04 '18 at 11:29

Yuki

3,857
5
25
43

I used touch File_name.txt to ensure that the times are different. So I'm skeptical that the problem relates to the file names/dates being the same. – Algorithmic Jul 04 '18 at 11:35

How do I Insert std::vector into a std::set which has a custom sort function

2 Answers2