Best way to move OwnedArray items into another OwnedArray?

General JUCE discussion
1

What is the best way to MOVE all the items from one OwnedArray and insert them into the middle of another OwnedArray? The insert() function only inserts one item, and the insertArray() requires a const array of the desired type. I’d like to do it efficiently, and copying each item from array 1, and inserting it into the middle of array 2, then doing that for each of the items in array 1, seems very INefficient! It has to insert in the middle many times, which I’d like to avoid if possible. Is there a way to do this more efficiently? Maybe copy all the items into an array, then use insertArray() is the best solution?

2

OwnedArray holds just pointers, so copying those around shouldn’t be a big deal, unless you are working with a huge number of elements. (Note by the way that you have to always keep things so that the same pointer is never left in multiple OwnedArrays, because you’d get multiple deletions of the object that way.)

3

But every insert() has to move all the items after it to the right, and potentially re-allocate the array, right? That’s the inefficiency I want to try to avoid. My lists are likely to have hundreds of pointers.

4

Yes, it’s algorithmically inefficient in theory to do that, but if you are working with just a few hundred elements, it’s unlikely you are going to see any impact in practice, especially if you do the operation only occasionally.

5

Hmm, ok. It’s the old programmer in me that bristles at the idea. I also find it odd that there is an addArray() function that takes another OwnedArray as an argument, but no matching insertArray. The insertArray() function expects a const array of the items to add, not another OwnedArray. I just wanted to make that one call, if possible. If not, I can do it manually, I guess. Thanks!

6
  • You can call removeAndReturn on an OwnedArray to remove an object from the list without deleting it.
  • You can use ensureStorageAllocated to ensure the size of the target container is large enough to contain all the incoming elements. This should mean any reallocation of memory will only happen once

Possible implementation:

template <typename T>
void moveElements (juce::OwnedArray<T>& source,
                   juce::OwnedArray<T>& target,
                   int insertionIndexInTarget = 0)
{
    const auto totalNumElements = source.size() + target.size();
    target.ensureStorageAllocated (totalNumElements);

    while (source.size() > 0)
    {
        target.insert (insertionIndexInTarget++,
                       source.removeAndReturn (0));
    }
}

Or, to avoid the overhead of moving the items in the target everytime you insert something:

template <typename T>
void moveElements (juce::OwnedArray<T>& source,
                   juce::OwnedArray<T>& target,
                   int insertionIndexInTarget = 0)
{
    juce::Array<T*> rawSource;

    while (source.size() > 0)
        rawSource.add (source.removeAndReturn (0));

    target.insertArray (insertionIndexInTarget,
                        rawSource.data(),
                        rawSource.size());
}
1 Like
7

I like that second one a lot! Thanks!

8

No guarantees it actually works, I didn’t test it :wink:

9

Works great!

1 Like
10

A quick remark (nitpicking):

    while (source.size() > 0)
        rawSource.add (source.removeAndReturn (0));

You could traverse (and remove) the source from end to start (to avoid to fill the gap each time).

And, probably use ensureStorageAllocated() on rawSource first?

1 Like
11

Yeah good point!

In fact, looking at it again, you might be able to do something like:

juce::Array<T*> rawSource {source.data(), source.size()};
source.clear (false);

Which takes a copy of all the raw pointers in the array, then clears the source array without deleting.

1 Like
12

In fact, you could probably just do:

template <typename T>
void moveElements (juce::OwnedArray<T>& source,
                   juce::OwnedArray<T>& target,
                   int insertionIndexInTarget = 0)
{
    target.insertArray (insertionIndexInTarget, source.data(), source.size());
    source.clear (false);
}
2 Likes
13

Read the documentation again and again is the JUCE’s user punishment. :grin:

2 Likes
14

So, basically, the answer to my question was “pass source.data(), not source, to insertArray()”. Odd that addArray() accepts the array, but insertArray() requires the data pointer itself, when the only real difference is that one appends and the other inserts.

15

Seems to me a good reason to ditch the OwnedArray and use std::vector<std::unique_ptr<T>>.

Messing with the raw data of the underlying array seems to defeat the purpose of RAII.
Although this solution will probably work and is in only few lines of code, I wouldn’t like that in my codebase. Just my opinion.

1 Like
16

What makes the STL approach better? Can you just std::move the vector or something?

Seems like it’d be the same issue of breaking RAII no matter what container you choose.

17

Yes you can:
https://en.cppreference.com/w/cpp/algorithm/move

template< class InputIt, class OutputIt >
OutputIt move( InputIt first, InputIt last, OutputIt d_first );

But moreover in the depicted example above, when accessing the raw memory of the array you create the situation that the ownership is ambiguous. By a future refactor, an exception or a incorrectly resolved merge conflict this could become a bug. It is a smelly workaround.

2 Likes
18

I found that interesting:

1 Like