0

I'm having a factory method object that creates a map as follows:

// std namespace is imported

Foo* createFoo() {
   map<int,int>* fooMap = new map<int,int>();
   for (int i=0;i < 4;i++) {
      fooMap->insert(make_pair(i+1, i+2));
   }
   return new Foo(fooMap);
}

The foo class is as follows:

class Foo { 
    private: 
        map<int,int>* m_fooMap; 
    public: 
        Foo(map<int,int>* fooMap) : m_fooMap(fooMap) { }; 
        void doIt() {
            cout << m_fooMap->at(1) << endl;
        }
}

This seems to throw an exception if I call the doIt function. When I debugged I noticed that the map object seems to not get created and populated. How can I correctly create a map on the heap?

PS: I don't want to create the map and pass by value, I'd prefer to do it through a pointer as a learning exercise. Also, if I create the map on the stack it gets populated, but of course I can't pass it to the Foo objects since it goes out of scope.

dev_nut
  • 2,476
  • 4
  • 29
  • 49
  • Why all the dynamic allocation? Just allocate automatic objects and return by value. – juanchopanza May 30 '13 at 15:09
  • I'd like to learn how to do it properly. I know passing by value will just work, but I'd like to understand why this doesn't work. – dev_nut May 30 '13 at 15:10
  • 6
    To do it properly is to avoid all these dynamic allocations. You are learning *not* to do it properly. – juanchopanza May 30 '13 at 15:11
  • Please present a complete code example that demonstrates the problem. Describe your output, and how it differs from your expectations. – Benjamin Lindley May 30 '13 at 15:11
  • You need to show the code using Foo, everything you've posted here is fine (although as above, you don't need to allocate Foo on the heap, the whole object is the same size as a pointer anyway) – Salgar May 30 '13 at 15:12
  • 1
    @Salgar there is a memory leak though, so it is not all completely fine. – juanchopanza May 30 '13 at 15:14
  • I edited the question again... I guess I want to learn to write efficient C++, so can I ask isn't passing by value a performance hit? – dev_nut May 30 '13 at 15:19
  • 2
    @dev_nut: [Not necessarily](http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/). – Jerry Coffin May 30 '13 at 15:20

1 Answers1

1

I would favour an approach without any explicit dynamic memory allocation:

class Foo { 
    private: 
        std::map<int,int> m_fooMap; 
    public: 
        Foo(const std::map<int,int>& fooMap) : m_fooMap(fooMap) {}; 
        Foo(std::map<int,int>&& fooMap) : m_fooMap(std::move(fooMap)) {}; 
        void doIt() {
            cout << m_fooMap.at(1) << endl;
        }
};

Foo createFoo() 
{
   std::map<int,int> fooMap;
   for (int i=0;i < 4;i++) {
      fooMap.insert(make_pair(i+1, i+2));
   }
   return Foo(fooMap);
}
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • Except this changes the signature and he said he wants to do it dynamically with pointers. – UpAndAdam May 30 '13 at 19:34
  • @UpAndAdam Correct, but I think OP's requirements are misguided. – juanchopanza May 30 '13 at 20:45
  • I know how to do this myself, I'm just making the point that you are sidestepping the entire issue of his question by changing that constraint. In any event classic Gang-of-Four factory methods are always going to need to return a pointer. An important part of the polymorphic purpose of a the factory method is to decouple knoweldge and how to of complex object creation from the calling/using classes. If he has to know what concrete type to utilize from the getgo you can't do this. http://stackoverflow.com/questions/1031301/can-i-implement-the-factory-method-pattern-in-c-without-using-new – UpAndAdam May 31 '13 at 15:55