STL Map container question?

[Patch97236]

I'm trying to plan a homework project where I have a database class and a report class, the report class will generate various reports using the data from the database class. The database class will be implemented with multiple nested classes that map key-values together using STL Maps. Now here's my question:

Is it bad programming practice to return an STL Map iterator reference(or is it pointer?) from the database class to the report class, so it can generate various reports?

For example:

  Class A {

  public:
   typedef std::maps<std::string, int> aMap;
   aMap example;

   aMap::iterator & getIterator() { return example.begin() }

   };

   Class B {

   public:
    A a;
    typedef std::maps<std::string, int> aMap;
    aMap example;
    aMap::iterator it = a.getIterator();

    void generateReport(){
    //generate report using iterator
    }

   };

[Repeater]

aMap::iterator & getIterator() { return example.begin() }

The iterator created here will cease to exist as soon as the function ends, and the returned reference will be a reference to something that no longer exists. This is bad news.

[keskiverto]

How would you "use" such iterator? Give a simple example.

[MikeyBoy]

Is it bad programming practice to return an STL Map iterator reference(or is it pointer?) from the database class to the report class, so it can generate various reports?

Firstly, it's a reference, not a pointer. And if you don't know the difference, then you shouldn't be using either. You should absolutely make it your first priority to learn what pointers are, and learn what references are, and learn the difference.

As Repeater has mentioned, this reference will be referring to something that has already been destroyed, so you won't be able to use it. (This would not be true if it were a const reference, because making a const reference to a temporary object extends the life of that object. But, obviously, making it const restricts what you can do with it.)

Why do you want to return a reference? What's wrong with simply returning a copy of the iterator?

[JLBorges]

The attempt to return an lvalue reference (to non-const) fails because it will not compile.

#include <map>
#include <string>

class A {

  public:
   typedef std::map<std::string, int> aMap;
   aMap example;

   // *** error: can't bind lvalue reference to non-const to an rvalue
   aMap::iterator& getIterator() { return example.begin() }

};

[Patch97236]

I guess I didn't realize the scope of the returned iterator from example.begin, I'm then assuming that returning a copy will work as so:

class A {

  public:
   typedef std::map<std::string, int> aMap;
   aMap example;

   aMap::iterator getIterator() { return example.begin() }
   aMap::iterator getEndIterator() {return example.end() }

};

The reason for returning an iterator for STL map in class A (the database class) is because class B will print to screen various reports on the data contained within multiple map containers in class A. So for class B to print these reports it needs access to the data in Class A. Something like this:

   Class B {

   public:
    //declare an object of class A
    A a;

     void generateReport(){
       for(std::map<std::string, int>::iterator it = a.getIterator(); it! = a.getEndIterator(); ++it) {
                std::cout << it->first << " => " << it ->second << '\n';
          }

     }

   };

So what I'm trying to figure out is this, does it go against the spirit of OOD to have code like this, where class B has access to class A(database) stl map containers through iterators?

There will be large volumes of data contained within an instance of class A, and I'm trying to figure out what would be the best approach for class B to have access to that data, which it will print to screen as a formatted report.

This is a homework project and we are restricted to having class A as a purely data-driven database and class B to generate various reports from the database.

[JLBorges]

To generate reports, we not not need to modify the map.
We can either provide const iterators or a reference to const map.

For example:

#include <map>
#include <string>

class A {

    public:
        typedef std::map<std::string, int> map_type;

        map_type::const_iterator get_begin() const { return example.cbegin() ; }
        map_type::const_iterator get_end() const { return example.cend() ; }

        const map_type& get_map() const { return example ; }

    private: map_type example;
};

struct B {

     void generate_report( const A& a ) const {

         for( auto iter = a.get_begin() ; iter != a.get_end() ; ++iter )
            std::cout << iter->first << " => " << iter->second << '\n';
     }

     void generate_report_2( const A& a ) const {

         for( const auto& pair : a.get_map() )
            std::cout << pair.first << " => " << pair.second << '\n';
     }
};

[Patch97236]

Would this sort of implementation violate the principle of data hiding for OOD? In other words, does returning a const iterator or const reference to map violate the principle of data hiding?

[Patch97236]

Our professor is a stickler about data hiding, so I'm worried about going this route.

[Thomas1965]

A different idea:

#include <iostream>
#include <map>
#include <string>

using namespace std;

class A
{
  map<string, int> data;
public:
  A()
  {
    data["key1"] = 1;
    data["key2"] = 2;
  }
  template<class Func>
  void for_each(Func func)
  {
    for (auto& kvp: data)
    {
      func(kvp.first, kvp.second);
    }
  }
};

class Report
{
public:
  void operator()(const string& key, int val)
  {
    cout << key << '\t' << val << '\n';
  }
};

int main() 
{
  A a;
  a.for_each(Report());
}

[keskiverto]

Would this sort of implementation violate the principle of data hiding for OOD? In other words, does returning a const iterator or const reference to map violate the principle of data hiding?

The idea of returning an iterator was yours. The const limits the access via the iterator, i.e. is "hiding" more.

Overall, if you do hand out map<string, int>::iterator to client, the client sees that you have at least map<string, int>. A relational database could have multiple tables (maps) and a query can combine data from those tables.


In that sense the Thomas Approach is superior. The class Report has no idea how the class A stores its data. The Report simply gets pieces of data for output formatting.

The A could have "premade queries":

class A
{
  vector<T> q1( const string& key );
  // more q's
}

Each query generates a list of results somehow. The user (B) does not know how data was copied to the result. The Ts could be int, tuple, ...

[Patch97236]

Yeah I know it was mines but I honestly wasn't sure if it was a good idea since I'm still trying to grasp the concept of OOD. Thank you guys for your replies, now I have a better idea of where I stand and which direction to go.

Thanks again, this site is such a big help for us beginners!