I just spent quite a long time tracking down an error where I was writing array[i] = someValue; for a JUCE array.
Of course, this code silently fails to do anything useful.
It’s a slight shame that JUCE doesn’t let you assign into arrays like this, because JUCE is the only thing in my world that doesn’t let you do that (and that’s covering C++'s STL, Python, Javascript, Java and even PHP when I can’t avoid it).
But it’s far too late to change now, and it’s no big deal writing a little more code except that this encourages you to write code that looks completely correct but is in fact completely wrong (and that you can look at many times and not realize is wrong).
Could juce::Array not be changed so that operator[] returns a const ElementType? Heck, I learned that trick from reading JUCE code!
I also agree. But the array class was one of the first classes that I wrote back in the last millenium, and I wasn’t quite such a C++ ninja back then. If I’d written it now, it’d certainly be very different, but to change it now would break a lot of dependant code.
The mistake I made was in saying that it’s ok to call operator with an out-of-bounds array index, which means that it can’t return a reference, because for invalid indexes, it’ll need to return a new default object. In my own code, I never use any more, I always use .getReference(), and I wish I could make operator work like that without repercussions.
That’s how it used to work, and that was indeed considered best practice until C++11 came along. Nowadays, you’re always supposed to return non-const objects, so that they can have their move operator applied if the type supports that.
Ah, this explains it - I thought you were returning a const at some point.
So there’s a choice - you can return a const and have safety; or, you can return non-const and have move semantics.
Personally, I’d vote for safety. Indeed, I don’t really see that “move semantics” will help you when returning a copy of an array element. There still has to be one copy; and I’d guess the optimizer should be able to eliminate any second copy that happens - though, the return value optimization can’t go off because (again) you’re returning that empty element if you’re out of range.
But I’d still vote for safety. If you really care about efficiency, you could use getReference()…
[quote=“TomSwirly”]So there’s a choice - you can return a const and have safety; or, you can return non-const and have move semantics.
Personally, I’d vote for safety.[/quote]
Thinking about it again now, you do have a good point. In almost all other circumstances, the reverse is true, so my general policy is to just automatically choose non-const return types, but Array may be a special case where it’s better not to.
If I had some spare time, I’d look at the generated code and see if the optimizer was really able to get rid of the second copy. I have managed to present to myself convincing arguments both ways.
You might have to change the code a little to make the optimization work:
[code] const ElementType operator[] (const int index) const
{
ElementType e;
const ScopedLockType lock (getLock());
if (isPositiveAndBelow (index, numUsed) )
e = data.elements [index];
return e;
}
well of course, I didn’t mean to sound pedantic! it’s just that I always forget which way is the good way and I used your Wiki as a reference, but in this particular case I remembered having read somewhere in the forum (i.e. this thread) about a change in the best practise for the const-ness or returned objects with value semantics.
I have a class (let’s call it a StringList) which is basically a wrapper around a StringArray that can be initialised parsing a string according to particular rules. Given the fact that the constructor with a single string as an argument already has another meaning in this context, would you add a
static StringList fromString (const String& s)
or would you add a boolean argument to the “single string” constructor, in order to choose which behaviour must apply to the string?
Constructors suck in every language, as you have to encode all the information about which constructor you are calling by the order and type of the construction parameters.
Of course, most of the time this is not an issue because you only have a few constructors and their meaning is clear, but the moment it is you should, as you are doing, create a helper function.
Let me note that if you’re dealing with something like a “StringList” it’s often better to have an empty constructor and then something that adds to the StringList from a String - because you’ll often be adding successive Strings to the end of your StringList in practice.
Yes, of course it’s done that way, and it has both an empty constructor and methods to add more strings to an existing list. The fromString method (or the single string argument constructor) is there to de-serialize the list from a string format it may have been previously written to with a toString() method. I need to do that because it is in fact travelling as the String argument for an ActionListener callback, thus in the actionListenerCallback(const String& message) I simply create my StringList back with
StringList list = StringList::fromString (message);
