CODEWITHSUNDEEP
c++
Seb
Seb

Reputation: 3

std::map: wrong number of template arguments, and assignment of read-only location

I am learning design patterns in C++. Now I am trying to implement Abstract Factory pattern. I have the following error related with std::map. I want to have a map with FactoryType as a key and unique_ptr to this factory type as a value. Currently I am facing multiple issues which origin I don't understand. Could you explain me what is wrong?

inc/GameObject/GameObjectFactory.hpp:13:59: error: wrong number of template arguments (1, should be at least 2)
                  std::unique_ptr<AbstractGameObjectFactory>> FactoriesArray;
                                                           ^~
/usr/include/c++/8/bits/stl_map.h:100:11: note: provided for ‘template<class _Key, class _Tp, class _Compare, class _Alloc> class std::map’
     class map
           ^~~

inc/GameObject/GameObjectFactory.hpp: In constructor ‘GameObjectFactory::GameObjectFactory()’:
inc/GameObject/GameObjectFactory.hpp:25:67: error: assignment of read-only location ‘"Player"[((GameObjectFactory*)this)->GameObjectFactory::m_factories]’
     m_factories["Player"] = std::make_unique<PlayerObjectFactory>();
                                                                   ^
inc/GameObject/GameObjectFactory.hpp: In member function ‘std::unique_ptr<GameObject> GameObjectFactory::create(std::__cxx11::string)’:
inc/GameObject/GameObjectFactory.hpp:30:27: error: no match for ‘operator[]’ (operand types are ‘FactoriesArray’ {aka ‘int’} and ‘const string’ {aka ‘const std::__cxx11::basic_string<char>’})
     auto obj = m_factories[type]->create();

Below is the code causing the issue:

#include <map>
#include <memory>
#include <string>
#include "GameObject/AbstractGameObjectFactory.hpp"
#include "GameObject/PlayerObjectFactory.hpp"


typedef std::map<std::string type, 
                 std::unique_ptr<AbstractGameObjectFactory>> FactoriesArray;

class GameObjectFactory
{
private:
  FactoriesArray m_factories;

protected:

public:
  GameObjectFactory()
  {
    m_factories["Player"] = std::make_unique<PlayerObjectFactory>();
  }
  
  std::unique_ptr<GameObject> create(const std::string type)
  {
    auto obj = m_factories[type]->create();
    return std::move(obj);
  }
};

Upvotes: 0

Views: 544

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 595742

Your typedef is slightly malformed. You need to remove type from std::string type, like this:

typedef std::map<std::string, 
                 std::unique_ptr<AbstractGameObjectFactory>> FactoriesArray;

Alternatively, you can use a type alias instead of a typedef:

using FactoriesArray = std::map<std::string, 
                                std::unique_ptr<AbstractGameObjectFactory>>;

On a separate note, you don't need the obj variable in your create() method:

std::unique_ptr<GameObject> create(const std::string type)
{
  return m_factories[type]->create();
}

But, as @Jarod42 mentioned in comments, this can be risky, because if the requested type is not in the map then m_factories[type] will return a new std::unique_ptr that is holding a nullptr, thus calling ->create() will have undefined behavior. You need to validate that the type exists, eg:

std::unique_ptr<GameObject> create(const std::string type)
{
  auto iter = m_factories.find(type);
  if (iter != m_factories.end())
    return iter->create();
  return nullptr; // or throw something...
}

Or:

std::unique_ptr<GameObject> create(const std::string type)
{
  return m_factories.at(type)->create();
}

std::map::at() will throw a std::out_of_range exception if the requested type is not found.

Upvotes: 2

Related Questions