Sorting Arrays of Objects

Hi there,

I have a class like this:
class Thing {
public:
int value1;
int value2;

and I have an array of things:

Array<Thing> things;

How can I sort this array by value1?

Many thanks!

It‘s described in the docs: https://docs.juce.com/master/classArray.html#a526081df81183ddd47f9f514de483a8f

You need to create your own comparison method, which is quite simple in this case.

1 Like

It would be a nice challenge to create a generic ElementComparator that takes a lambda nowadays… (or add it directly to the Array class)

1 Like

https://docs.juce.com/master/tutorial_table_list_box.html
In this exemple a sort xml data can be helpful.

Array works with std::sort/std::stable_sort and lambdas :

std::sort(arr.begin(), arr.end(), 
  [](const auto& lhs, const auto& rhs) { return lhs.value1 < rhs.value1; });
5 Likes

Thanks for the replies. I did look at the documentation, but it doesn’t provide any examples. I have tried this:

class MyComparator
{
public:
    static int compareElements (Thing* first, Thing* second)
    {
        if (first->value1 < second->value1)
            return -1;
        else if (first->value1 > second->value1)
            return 1;
        else
            return 0; //items are equal
    }
};

and tried using it like this:

Array<Thing> things;
MyComparator sorter;
things.sort(sorter);

Which gives me this error:

Error:(13) no matching function for call to object of type ‘juce::SortFunctionConverter’

What am I doing wrong?

Your Array has Thing as ParameterType, so the compareElements will have to look like:

static int compareElements (Thing first, Thing second)
    {
        if (first.value1 < second.value1)
            return -1;
        else if (first.value1 > second.value1)
            return 1;
        else
            return 0; //items are equal
    }

But the call has implications, calling with an argument Thing will create a copy of first and second on the method’s argument stack.

Passing Thing by const-ref should also match and won’t incur any copy

    static int compareElements (const Thing& first, const Thing& second)
    {
        if (first.value1 < second.value1)
            return -1;
        if (first.value1 > second.value1)
            return 1;
        return 0;
    }

I was wondering, if it does. Also with the const?

Especially with the const!

In the following example, A, B, and C are viable candidates, and the compiler will fail complaining that it is ambiguous:

void f(int) {} // A
void f(int&) {} // B
void f(const int&) {} // C
void f(int&&) {} // D
void g() { int i = 23; f(i); }

In the following example, A, C and D are viable candidates, and the compiler will fail complaining that it is ambiguous:

void f(int) {} // A
void f(int&) {} // B
void f(const int&) {} // C
void f(int&&) {} // D
void g() { f(42); }
1 Like
    static int compareElements (Thing* first, Thing* second)
    {
        if (first->value1 < second->value1)
            return -1;
        else if (first->value1 > second->value1)
            return 1;
        else
            return 0; //items are equal
    }

Coding style question…

Why else ??

Rail

2 Likes

The else just feels cleaner to me. Does it have a cost do you think?

For reference, McMartin had the solution. So to soft an array:

class Thing {
  public:
  int value1;
}

class MyComparator {
public:
    static int compareElements (const Thing& first, const Thing& second)
    {
        if (first.value1 < second.value1)
            return -1;
        if (first.value1 > second.value1)
            return 1;
        return 0;
    }
  };

and to actually sort it:

Array<Thing> things; 
MyComparator sorter; 
things.sort(sorter);

Thank you everybody.

It adds unnecessary words to read and potentially additional indentation. I doubt it will add processing cost.

See the explanations on llvm Coding Standards or Jules’ advice on JUCE - Coding Standards (scroll to Miscellaneous).

I’m new to C++, and I’ll never be at the level of some of you guys, but I can’t say I agree with that particular coding standard. It means that you have to read every line in order to understand what is happening.

This pattern has 3 possible outcomes, it’s very clear what’s going to happen:

if (sometest)
    do something
else if (sometest)
    do something
else
    do something

This pattern could do a number of things depending on what the the 'do something’s are:

if (sometest)
    do something
if (sometest)
    do something

do something

If the ‘do something’ are all returns, it is functionally the same as the first pattern:

if (sometest)
    return
if (sometest)
    return

return

But if they are something else, the flow is completely different:

if (sometest)
    value + 2
if (sometest)
    value + 2

value + 2

In other words, you have to read every line in order to understand the flow, you can’t just glance at it. I don’t see how that’s an improvement… what am I missing?

Read http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

If control flow is broken, then the else is irrelevant… and actually makes the code harder to read.

Rail

…and that’s the reason, why you should leave an empty line after a return, so that you really see at first glance: “This function is done here!”

Burying returns in the middle of full lines is indeed hiding the return.

When skimming over a function, a return should definitely catch my eye, which makes the else superfluous and I can focus on the next condition.

1 Like

More importantly for me, is the need for break-point lines.

Sadly this is no longer working, something was changed inside JUCE and the whole thing is a mess now. :frowning:

class WLoopPart
{
public:
	WLoopPart() : position(0.0f), value(1.0f) { };
	//
	float position, value;
};
//
// ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
class WLoopPartElementComparator
{
public:
	static int compareElements(const WLoopPart& first, const WLoopPart& second)
	{
		return (first.position < second.position) ? -1 : ((second.position < first.position) ? 1 : 0);
	}
};

containers\juce_elementcomparator.h(40): note: Reason: cannot convert from ‘Type’ to ‘const WLoopPart’