I currently have this comparator function that sorts a custom struct using qsort, and I want to port it to std::sort, but it seems like it needs to return a bool value instead of -1, 0 and 1, how would I rewrite it?

int tsort = 1, sorta = 1;

static auto titleSort(const void *c1, const void *c2) -> int {
    switch (tsort) {
        case 0:
            return ((Title *) c1)->listID - ((Title *) c2)->listID;

        case 1:
            return strcmp(((Title *) c1)->shortName, ((Title *) c2)->shortName) * sorta;

        case 2:
            if (((Title *) c1)->isTitleOnUSB == ((Title *) c2)->isTitleOnUSB)
                return 0;
            if (((Title *) c1)->isTitleOnUSB)
                return -1 * sorta;
            if (((Title *) c2)->isTitleOnUSB)
                return 1 * sorta;
            return 0;

        case 3:
            if (((Title *) c1)->isTitleOnUSB && !((Title *) c2)->isTitleOnUSB)
                return -1 * sorta;
            if (!((Title *) c1)->isTitleOnUSB && ((Title *) c2)->isTitleOnUSB)
                return 1 * sorta;

            return strcmp(((Title *) c1)->shortName, ((Title *) c2)->shortName) * sorta;

        default:
            return 0;
    }
}

CodePudding user response：

As mentioned in a comment, the simple, quick, and dirty solution is

std::sort(first,last, [](const auto& a,const auto& b) { 
          return titleSort(&a,&b) < 0; 
});

Because the comparator for std::sort should return true when a < b and false otherwise. When a < b then titleSort returns -1 and 0 or 1 otherwise. Hence you want to map 1 to true and 0 and 1 to false, and < 0 does that.

You also need to consider that the comparator for std::sort must conform to the named requirement Compare. Typical comparisons do that, but when they don't then using them with std::sort is undefined. I didn't find similar requirement for qsort.

CodePudding user response：

If possible, get rid of int tsort = 1, sorta = 1;

and use directly the appropriate method.

The dispatch, if needed, can be done with something like (C 20):

void sortTitle(std::vector<Title>& title, int tsort = 1, sorta = 1)
{
    switch (tsort)
    {
        case 0:
            std::ranges::sort(titles, std::ranges::less{}, &Title::listID); break;
        case 1: {
            const auto proj = [](const Title& title){
                return std::string_view(title.shortName);
            };
            if (sorta == 1) {
                std::ranges::sort(titles, std::ranges::less{}, proj);
            } else {
                std::ranges::sort(titles, std::ranges::greater{}, proj);
            }
            break;
        }
        case 2:
            if (sorta == 1) {
                std::ranges::sort(titles, std::ranges::less{}, &Title::isTitleOnUSB);
            } else {
                std::ranges::sort(titles, std::ranges::greater{}, &Title::isTitleOnUSB);
            }
            break;
        case 3: {
            const auto proj = [](const Title& title){
                return std::make_tuple(title.isTitleOnUSB,
                                       std::string_view(title.shortName));
            };
            if (sorta == 1) {
                std::ranges::sort(titles, std::ranges::less{}, proj);
            } else {
                std::ranges::sort(titles, std::ranges::greater{}, proj);
            }
            break;
        }
    }
}

CodePudding user response：

The pragmatic approach has a lot of upsides. You don't need to change much for example.

However, the old titleSort did a lot of runtime checking of the tsort and sorta values. It would be better if the runtime values could be checked one time only (in runtime).

You could create a functor template that you instantiate with tsort and sorta as template parameters. If you'd like to keep all the logic in one place as before, you'd use constexpr-if to make the selection:

enum class tsort_t { zero, one, two, three };

template <tsort_t tsort, bool sorta = false>
struct titleSorter {
    bool operator()(const Title& a, const Title& b) const {
        if constexpr (tsort == tsort_t::zero) {
            return b.listID < a.listID;

        } else if constexpr (tsort == tsort_t::one) {
            return std::strcmp(a.shortName, b.shortName) < 0;

        } else if constexpr (tsort == tsort_t::two) {
            if (a.isTitleOnUSB == b.isTitleOnUSB) return false;
            if constexpr (sorta) {
                return a.isTitleOnUSB;
            } else {
                return b.isTitleOnUSB;
            }
        } else if constexpr (tsort == tsort_t::three) {
            if (a.isTitleOnUSB != b.isTitleOnUSB) {
                if constexpr (sorta) {
                    return a.isTitleOnUSB;
                } else {
                    return b.isTitleOnUSB;
                }
            } else {
                return strcmp(a.shortName, b.shortName) < 0;
            }
        }
    }
};

^{Disclaimer: I may have made some mistakes when converting the logic, but I think it's ok - double check it}

And for checking the runtime values of tsort and sorta, you'd add a frontend which instantiates the proper titleSorter:

template <class It>
void titleSort(It first, It last, int tsort, int sorta) {
    switch (tsort) {
        case 0:
            std::sort(first, last, titleSorter<tsort_t::zero>{});
            break;
        case 1:
            if (sorta == 1)
                std::sort(first, last, titleSorter<tsort_t::one, true>{});
            else
                std::sort(first, last, titleSorter<tsort_t::one>{});
            break;
        case 2:
            if (sorta == 1)
                std::sort(first, last, titleSorter<tsort_t::two, true>{});
            else
                std::sort(first, last, titleSorter<tsort_t::two>{});
            break;
        case 3:
            if (sorta == 1)
                std::sort(first, last, titleSorter<tsort_t::three, true>{});
            else
                std::sort(first, last, titleSorter<tsort_t::three>{});
            break;
        default:
            throw std::runtime_error("erroneous tsort algorithm selected");
    }
}

Demo

Note: I would also consider changing shortName to being a std::string.