1

I have the following setup:

A.h

class A
{
public:
    ...
    const std::vector<int>& getSomeData() const;

private:
    std::map<SomeIdType, std::shared_ptr<SomeDataType> > m_someDataCollection;
}

A.cpp

const std::vector<int>& getSomeData(SomeIdType id) const
{
    std::vector<int> rVal;

    auto it = m_someDataCollection.find(id);
    if (it != m_someDataCollection.end())
    {
        std::shared_ptr<SomeDataType> someData = it->second;
        rVal = someData->getSomeDataVector(); // returns std::vector<int>
    }
    return  rVal;
}

int main()
{
    SomeIdType id;
    A a;
    ...
    const std::vector<int>& data = a.getSomeData(id);
}

There is a reference to a local variable returned in A.cpp.

I don't want the vector returned by A::getSomeData() to be manipulated outside.

How can I achieve this goal, without having to return a reference to a local variable in A.cpp?

Improve this question
8
  • Use std::vector<int> getSomeData(SomeIdType id) const. whereas user can still modify returned vector, that doesn't affect the current class instance. Commented Feb 6, 2020 at 12:40
  • If the id is not found in m_someDataCollection, it should return a reference to an empty vector, right? Commented Feb 6, 2020 at 12:41
  • @Jarod42 that will copy the entire vector (usually). Commented Feb 6, 2020 at 12:41
  • @user253751: "someData->getSomeDataVector(); // returns std::vector<int>" that already does copy. Commented Feb 6, 2020 at 12:42
  • 1
    @user253751 The vector will be moved (return id-expression; naming a local variable is a move context, and a copy elision context) Commented Feb 6, 2020 at 12:54

4 Answers 4

Reset to default
4

How to return const reference of a vector?

  1. By creating the vector somewhere other than the function. Since it is a member function in this case, an option is to return reference to a member. Or the vector could be passed into the function through a parameter.
  2. Or by using a static local variable. In that case all invcations of the function would return the same vector.

Any non-static local variable don't want the vector returned by A::getSomeData() to be manipulated outside.e has automatic storage and therefore exists only until the function ends. A returned reference to such variable will be referring to a destroyed vector.

That said, it looks like from your example that you should probably return the vector by value instead of trying to return a reference.

I don't want the vector returned by A::getSomeData() to be manipulated outside.

You haven't given a reason for why you would want that. Why should you care what the caller does with the vector that you've returned?

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

I assume that when the id is found in the data, you want to return a reference to that id's vector instead of copying the entire vector.

For the empty case, you could return a reference to a static local variable which is always empty:

const std::vector<int>& getSomeData(SomeIdType id) const
{
    auto it = m_someDataCollection.find(id);
    if (it != m_someDataCollection.end())
    {
        return it->second->getSomeDataVector(); // returns const std::vector<int>&
    }
    else
    {
        static const std::vector<int> emptyVector;
        return emptyVector;
    }
}
Improve this answer

1 Comment

The question specified getSomeDataVector() as returning by value, so this will return a dangling reference
3

After you convert the values to int, they no longer refer to the original data, so it cannot be modified by the caller. It is safe to return the vector by value. If the vector were to contain pointers or references, it would not be safe.

std::vector<int> getSomeData(SomeIdType id) const
{
    auto it = m_someDataCollection.find(id);
    if (it != m_someDataCollection.end())
    {
        std::shared_ptr<SomeDataType> someData = it->second;
        return someData->getSomeDataVector(); // returns std::vector<int>
    }
    return {};
}

const auto data = a.getSomeData(id);
Improve this answer

5 Comments

@eerorika the return statement is a move context anyway so it's negligible overhead
now, return std::move(rVal); is definitely a bad idea
@M.M What is negligible overhead depends on context. Without context, you can only guess.
OP wants to return const reference but you are returning a copy
@NutCracker OP request is impractical so the answers are suggesting other ways to solve the problem OP is facing
1

If you are using C++17, one of the possibilities would also be to use std::optional. In case you have found an element with the specified ID, you would return std::vector. Otherwise, you would return std::nullopt. Here is how your code would in this case look like:

std::optional<std::vector<int>> getSomeData(SomeIdType id) const {
    auto it = m_someDataCollection.find(id);
    if (it != m_someDataCollection.end()) {
        return it->second->getSomeDataVector();
    }
    else {
        return {}; // or return std::nullopt;
    }
}

And then you could use it like this:

int main() {
    SomeIdType id;
    A a;

    // ...

    auto data = a.getSomeData(id);
    if (data) {
        // there is an item with the specified ID
        // example of accessing the std::vector
        for (auto const i : data.value().get()) {
            // more code here ....
        }
    } else {
        // there is NO item with the specified ID
    }
}
Improve this answer

2 Comments

getSomeDataVector() returns by value (according to code comments in the question -- the actual code was not shown) , so this will return a dangling reference_wrapper
@M.M did the OP really stated that? Haven't seen it. Sorry. It is fixed now. Thanks

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.