2

Lets say you have 2 vectors passed to a function as lvalue references. Later you realize, you can use recursion and pass slices of these vectors using their iterators.

Would it be an appropriate strategy if I go ahead and write some util function to use these vecotrs as rvalues ? Or I should avoid this in any way?

Simplified pattern:

Node* util(vector<int>&& a, vector<int>&& b) {
    Node* root = new Node(a[0]);

    root->left = util({ a.begin(), a.end() }, { b.begin(), b.end() });
    root->right = util({ a.begin(), a.end() }, { b.begin(), b.end() });

    return root;
}

Node* main(vector<int>& a, vector<int>& b) {
    return util({ a.begin(), a.end() }, { b.begin(), b.end() });
}

Realistic example (LeetCode 105):

TreeNode* util(vector<int>&& preorder, vector<int>&& inorder) {
    if (!preorder.empty()) {
        TreeNode* root = new TreeNode(preorder[0]);

        auto r = find(inorder.begin(), inorder.end(), preorder[0]);

        root->left = util(
        { preorder.begin() + 1, preorder.begin() + 1 + distance(inorder.begin(), r) },
        { inorder.begin(), r });
        root->right = util(
        { preorder.begin() + 1 + distance(inorder.begin(), r), preorder.end() },
        { r + 1, inorder.end() });

        return root;
    }
    return nullptr;
}

TreeNode* buildTree(vector<int>& preorder, vector<int>& inorder) {
    return util({ preorder.begin(), preorder.end() }, 
                { inorder.begin(), inorder.end() });
}
user2376997
  • 501
  • 6
  • 22
  • 2
    I don't see why you'd need r-value references (do you want to *move* any data???), ordinary references should be fine, too. As you don't seem to modify input vectors, const references would be appropriate. – Aconcagua Jul 29 '19 at 08:02
  • 1
    Doesn't this construct a whole new vector for each argument anyway? rvalues wouldn't stop that from happening – Tharwen Jul 29 '19 at 08:05
  • I can't pass vector iterators with ordinary references. This syntax `root->left = util({ a.begin(), a.end() }, { b.begin(), b.end() });` won't work – user2376997 Jul 29 '19 at 08:07
  • 1
    @TharwenWas just about to modify my previous comment in that direction, too... Yes, we're creating copies over copies without ever modifying them. So: const references as in my previous comment, and just call recursively `util(preorder, inorder);` should be absolutely fine. – Aconcagua Jul 29 '19 at 08:07
  • 4
    @user2376997 You can pass iterators by value; under the hoods, you are more or less just passing pointers into the data anyway... Provide for iterators as arguments to your util function and you can do so. `{a.begin(), a.end()}` is equivalent to `std::vector(a.begin(), a.end())`, i. e. calls the constructor accepting two iterators, which *copies* all data in between these two iterators, i. e. the whole vector in your case. – Aconcagua Jul 29 '19 at 08:09

1 Answers1

5

You are on the wrong way: {a.begin(), a.end()} is equivalent to std::vector<int>(a.begin(), a.end()), i. e. calls the constructor accepting two iterators, which copies all data in between these two iterators, i. e. the whole vector in your case (or sub-ranges of the vector in your more realistic example).

But you don't modify the vectors in any way, so no need for any copies.

If you just want to read the original vector, then you can work directly with iterators; you can pass these by value, unter the hoods, they are more or less just pointers into the vector's data anyway:

TreeNode* util
(
    vector<int>::const_iterator preorderBegin, vector<int>::const_iterator preorderEnd,
    vector<int>::const_iterator inorderBegin, vector<int>::const_iterator inorderEnd
)
{
    // ...
    ++preorderBegin; // if we do so before the call, we have just one addition; compiler
                     // might optimize to the same code, but we don't rely on it this way
    util(preorderBegin, preorderBegin + distance(), inorderBegin, r);
    // ...
}

const_iterator? Well, we want to accept const vectors:

TreeNode* buildTree(vector<int> const& preorder, vector<int> const& inorder)
//                                ^                            ^
// you don't modify the vectors, so accept them as constant
{
    return util(preorder.begin(), preorder.end(), inorder.begin(), inorder.end());
    // note the dropped braces...
}
Aconcagua
  • 24,880
  • 4
  • 34
  • 59