Sorting Arrays of Objects

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); }
    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’

Can you show the declaration of the Array and how you call sort, so people can check?

Just the basic stuff.

OwnedArray<WLoopPart> theArray;

WLoopPartElementComparator sorter
theArray.sort(sorter);

Something like that. I already erased the code and will do my own sorter. :frowning:

But thanks for the help. (hug)

I would assume OwnedArray contains pointers, so a comparison function/object that takes in references for the elements to compare isn’t going to work.

From the OwnedArray docs…

Sorts the elements in the array.

This will use a comparator object to sort the elements into order. The object passed must have a method of the form:

int compareElements (ElementType* first, ElementType* second);

(facepalm) (shameface) thanks guys… I’m getting old faster… :-p

1 Like