Correct way to return stl map by reference / Is this behavior well defined?

Question

I have 2 classes and one of them has map<string, vector<string> >I want to be able to use this in other class. Here is my code:

class a
{
    map<string, vector<string> > m;
    public:
    const map<string, vector<string> > & get()
    {
        return m;
    }
};

class b
{
    a obj;
    public:
    void test()
    {
        map<string, vector<string> > m= obj.get();
        // and then print
    }
};

int main(int argc, char ** argv)
{
    b bobj;
    bobj.test();
    return 0;
}

Is the way I returned reference to map in class a correct? It worked,but I just want to confirm if it was done properly / I got lucky / any other comments about the code.

Thank you for your help.

Answer 1

If you do not want to change the map in b::test(), you should not make a copy of it:

const map<string, vector<string> >& m = obj.get(); // note the const &

My objections:

1. Major: a::get() should be const:

   const map<string, vector<string> > & get() const // note the const at the end
   

2. Minor: I would create an alias for the map's type using typedef.

   typedef map<string, vector<string> > my_map_t;
   

3. Minor: I can't see at all what b is for.

Given these, my code would look like this:

class a
{
    public:
      typedef map<string, vector<string> > my_map_t;

      const my_map_t & get() const {return m;}

    private:
      my_map_t m;
};

void test(const a& obj)
{
    const a::my_map_t& m = obj.get();
    // and then print
}

int main()
{
    a obj;
    test(obj);
    return 0;
}

Answer 2

Yes, that is the correct way to return a reference to a constant object.

However, in your test function where you receive the returned reference, the left hand side is not a reference. That means you're actually going to create a copy of the entire map in this function. A simple change will fix that problem, and then the test function becomes zero copy:

void test()
{
    map< string, vector<string> > const & m = obj.get();
    // and then print
}

Answer 3

It is correct, but you should use a reference to store the result of obj.get(), as mentioned previously. It would have been incorrect if you would have done something like this:

const map< string, vector<string> >& foo()
{
    map< string, vector<string> > m;
    // ...
    return m;
}

because, in this case, m will not exist anymore after foo() completes execution.