Maze game from book Design Patterns with smart pointers and polymorphism

Question

The Design Patterns book chapter 3 about Creational Patterns starts with a maze game. They use the maze as an example throughout the chapter and instead of rooms they have also things like enchanted rooms and rooms with a bomb.

The book is from the 90s and it uses raw pointers. There is vector of rooms in the maze and they will be polymorphic so I have to use pointers.

I implemented it using smart pointers. Because of the cyclic structure of rooms and doors, I think I have to use shared_ptr and weak_ptr. I have only used unique_ptr before in my projects.

I would like to get feedback about pointers. I am not sure when I should use shared_ptr instead of unique_ptr. I think the maze could own all map sites and have a vector of unique_ptrs instead. Maybe give tips for both approaches.

I don't see any reason why the player or the maze should be pointers. Also, in the function Room& getRoom() const I return a reference instead of a smart pointer because it doesn't have to share ownership.

The example looks like this:

enum Direction {North, South, East, West};

class MapSite {
public:
  virtual void Enter() = 0;
};

class Room : public MapSite {
public:
  Room(int roomNo);
  MapSite* GetSide(Direction) const;
  void SetSide(Direction, MapSite*);
  virtual void Enter();
private:
  MapSite* _sides[4];
  int _roomNumber;

class Wall : public MapSite {
public:
  Wall();
  virtual void Enter();
};

class Door : public MapSite {
public:
  Door(Room* = 0, Room* = 0);
  virtual void Enter();
  Room* OtherSideFrom(Room*);
private:
  Room* _room1;
  Room* _room2;
  bool _isOpen;
};

class Maze {
public:
  Maze();
  void AddRoom(Room*);
  Room* RoomNo(int) const;
};

Maze* MazeGame::CreateMaze() {
  Maze* aMaze = new Maze;
  Room* r1 = new Room(1);
  Room* r2 = new Room(2);
  Door* theDoor = new Door(r1, r2);

  aMaze.AddRoom(r1);
  aMaze.AddRoom(r2);

  r1->SetSide(North, new Wall);
  r1->SetSide(East, theDoor);
  r1->SetSide(South, new Wall);
  r1->SetSide(West, new Wall);

  r2->SetSide(North, new Wall);
  r2->SetSide(East, new Wall);
  r2->SetSide(South, new Wall);
  r2->SetSide(West, theDoor);

  return aMaze;
}

Here is my implementation.

maze.h

#ifndef MAZE_H
#define MAZE_H

#include <vector>
#include <array>
#include <memory>

enum Direction { north, south, east, west };

class Player;


class MapSite {
  public:
    virtual ~MapSite() = default;
    virtual std::string enter(Player& player) = 0;
};

class Room : public MapSite {
  public:
    Room(int id);
    MapSite* getSide(Direction direction) const;
    void setSide(Direction direction, std::shared_ptr<MapSite> mapSite);
    std::string enter(Player& player) override;
    friend bool operator==(const Room& left, const Room& right);

  private:
    std::array<std::shared_ptr<MapSite>, 4> sides;
    int id;
};

class Wall : public MapSite {
  public:
    std::string enter(Player& player) override;
};

class Door : public MapSite {
  public:
    Door(std::shared_ptr<Room> room1, std::shared_ptr<Room> room2);
    std::string enter(Player& player) override;

  private:
    std::weak_ptr<Room> room1;
    std::weak_ptr<Room> room2;
};

class Maze {
  public:
    void addRoom(std::shared_ptr<Room> room);

  private:
    std::vector<std::shared_ptr<Room>> rooms;
};

class Player {
  public:
    Player(std::shared_ptr<Room> room);
    Room& getRoom() const;
    std::string enter(std::shared_ptr<Room> r);
    std::string move(Direction direction);

  private:
    std::shared_ptr<Room> room;
};

class MazeGame {
  public:
    MazeGame(Maze maze, Player player);
    std::string move(Direction direction);

  private:
    Maze maze;
    Player player;
};

MazeGame createMaze();

#endif

maze.cpp

#include <iostream>

#include "maze.h"

constexpr char nl = '\n';


Room::Room(int id): id(id) {}

MapSite* Room::getSide(Direction direction) const {
  return sides[direction].get();
}

void Room::setSide(Direction direction, std::shared_ptr<MapSite> mapSite) {
  sides[direction] = mapSite;
}

std::string Room::enter(Player& player) {
  return "entered room " + std::to_string(id);
}

bool operator==(const Room& left, const Room& right) {
  return left.id == right.id;
}


std::string Wall::enter(Player& player) {
  return "walked into a wall";
}


Door::Door(std::shared_ptr<Room> room1, std::shared_ptr<Room> room2):
  room1(std::move(room1)), room2(std::move(room2))
{}

std::string Door::enter(Player& player) {
  if (player.getRoom() == *room1.lock()) return player.enter(room2.lock());
  if (player.getRoom() == *room2.lock()) return player.enter(room1.lock());
  return "the door is locked";
}


void Maze::addRoom(std::shared_ptr<Room> room) {
  rooms.push_back(std::move(room));
}


Player::Player(std::shared_ptr<Room> room): room(room) {}

Room& Player::getRoom() const {
  return *room;
}

std::string Player::enter(std::shared_ptr<Room> r) {
  room = r;
  return room->enter(*this);
}

std::string Player::move(Direction direction) {
  return room->getSide(direction)->enter(*this);
}

MazeGame::MazeGame(Maze maze, Player player):
  maze(maze), player(player)
{}

std::string MazeGame::move(Direction direction) {
  return player.move(direction);
}


MazeGame createMaze() {
  Maze maze {};
  std::shared_ptr<Room> r1 = std::make_shared<Room>(1);
  std::shared_ptr<Room> r2 = std::make_shared<Room>(2);
  std::shared_ptr<Door> door = std::make_shared<Door>(r1, r2);

  maze.addRoom(r1);
  maze.addRoom(r2);

  r1->setSide(north, std::make_shared<Wall>());
  r1->setSide(east, door);
  r1->setSide(south, std::make_shared<Wall>());
  r1->setSide(west, std::make_shared<Wall>());

  r2->setSide(north, std::make_shared<Wall>());
  r2->setSide(east, std::make_shared<Wall>());
  r2->setSide(south, std::make_shared<Wall>());
  r2->setSide(west, door);

  return MazeGame { maze, Player { r1 } };
}

main.cpp

#include <iostream>

#include "maze.h"


constexpr char nl = '\n';

int main() {
  MazeGame mazeGame = createMaze();

  std::string line;
  std::cout << "> ";
  while (std::getline(std::cin, line)) {
    if (line.empty()) break;
    switch (line[0]) {
      case 'n':
        std::cout << mazeGame.move(north) << nl;
        break;
      case 'e':
        std::cout << mazeGame.move(east) << nl;
        break;
      case 's':
        std::cout << mazeGame.move(south) << nl;
        break;
      case 'w':
        std::cout << mazeGame.move(west) << nl;
        break;
    }
    std::cout << "> ";
  }
}

G. Sliepen · Accepted Answer · 2025-05-10 10:48:52Z

You follow good practices for inheritance

The "example" you show at the beginning uses some bad practices:

It doesn't make the destructor of MapSite virtual
It doesn't use override in the derived classes

But in your implementation, you are doing the correct thing, which is great!

Use of `std::shared_ptr`

I implemented it using smart pointers. Because of the cyclic structure of rooms and doors, I think I have to use shared_ptr and weak_ptr. I have only used unique_ptr before in my projects.

While that might at first sound like a good idea, it's not a great solution. std::shared_ptr is very safe, but it has issues:

It's not as efficient as std::unique_ptr.
It can result in memory leaks if you have std::shared_ptrs pointing to each other.

You narrowly avoid the latter because you only connect Rooms together via Doors, and Doors use std::weak_ptrs. But if you did:

r1->setSide(east, r2);
r2->setSide(west, r1);

Then you have created a cycle, and the memory for r1 and r2 would not be freed automatically anymore if you just deleted r1 and r2.

But in your code, you actually don't need std::shared_ptr. You already have Maze storing pointers to all the map sites, so you could have Maze be the sole owner of them. As long as you guarantee that Maze will never delete a map site that is still in use, you can safely use raw pointers in Room and Door.

Simplify adding rooms

I see in createMaze() that you still have to do several manual steps to add rooms to the maze. First of all, you have to manually create a room, and then in a separate step add it to the maze. That's more work, and leaves the possibility of forgetting the second step. Ideally, you have one function to do all that for you.

One possibilty would be to add a function to Maze to do this:

class Maze {
public:
    Room& addRoom(int id) {
        return rooms.emplace_back(id);
    }

private:
    std::vector<Room> rooms;
};

Then you can write:

MazeGame createMaze() {
  Maze maze;
  auto r1 = maze.addRoom(1);
  auto r2 = maze.addRoom(2);

  r1.setSide(north, …);
  …
}

Of course, you can't just do this for Room, you have to do this for all types of map sites. But that can be done as well:

class Maze {
public:
    Room& addRoom(int id) {
        sites.push_back(std::make_unique<Room>(id));
        return static_cast<Room&>(*sites.back());
    }

    Door& addDoor(Room& room1, Room& room2) {
        …
    }

    Wall& addWall() {
        …
    }

private:
    std::vector<std::unique_ptr<MapSite>> sites;
};

MazeGame createMaze() {
    Maze maze;
    auto r1 = maze.addRoom(1);
    auto r2 = maze.addRoom(2);
    auto door = maze.addDoor(r1, r2);

    r1.setSide(north, maze.addWall());
    r1.setSide(east, door);
  …
}

Now you can have the site types store raw pointers and/or references:

class Room : public MapSite {
public:
    Room(int id): id(id) {}
    void setSide(Direction direction, MapSite& mapSite) {
        sides[direction] = &mapSite;
    }
    …
private:
    std::array<MapSite*, 4> sides;
    int id;
};

class Door : public MapSite {
public:
    Door(Room& room1, Room& room2): room1(room1), room2(room2) {}
    …
private:
    Room& room1;
    Room& room2;
};

To avoid having to write a new addSomething() function every time you add a new map site type, you can make it a template:

class Maze {
public:
    template<typename T, typename... Args>
    T& add(Args&&... args) {
        sites.push_back(std::make_unique<T>(std::forward<Args>(args)...));
        return static_cast<T&>(*sites.back());
    }

private:
    std::vector<std::unique_ptr<MapSite>> sites;
};

MazeGame createMaze() {
    Maze maze;
    auto r1 = maze.add<Room>(1);
    auto r2 = maze.add<Room>(2);
    auto door = maze.add<Door>(r1, r2);

    r1.setSide(north, maze.add<Wall>());
    r1.setSide(east, door);
    …
}

It's easy to create invalid mazes

What if you do:

r1->setSide(east, door);
…
r2->setSide(south, door);

Now you enter room 2 from the east, but to get back to room 1 you have to go south. What if you forgot to call r2->setSide(…, door) at all? Then you can never leave room 2.

What if you forgot to write:

r1->setSide(north, std::make_shared<Wall>());

Now going north will result in your program crashing.

What if you have three rooms, r1, r2 and r3, and a door between r1 and r2, but then write:

r3->setSide(west, door);

Then that door will be permanently locked. Sure, it's not a crash, but it probably still is a bug.

What if you wrote:

r1->setSide(east, r2);
…
r2->setSide(west, r1);

Looks fine, just a connection between two rooms without a door inbetween. It looks like moving east from room 1 will bring you to room 2, but it only prints "entered room 2", it never updates the player's position.

I would try to make the code such that you can never create invalid mazes. Make sure rooms start out with four walls, that if you add a door, it will correctly set up the pointers in both the Door and the two Rooms it joins, and so on.

Room identifiers

You have to manually assign an id to each room. What if you give two rooms the same identifier? Sure, it doesn't cause a crash, but it will confuse the player. I would either:

Enforce that a unique identifier is provided, perhaps by having a std::unordered_map<int, Room*> to keep track of which room is associated with which identifier.
Automatically generate a unique identifier when adding a room to the Maze. You could add a counter variable to Maze, or perhaps you can just use the position in which the room is inserted into the vector rooms as the identifier.

Stack Exchange Network

Maze game from book Design Patterns with smart pointers and polymorphism

1 Answer 1

You follow good practices for inheritance

Use of `std::shared_ptr`

Simplify adding rooms

It's easy to create invalid mazes

Room identifiers

You must log in to answer this question.

Hot Network Questions

Maze game from book Design Patterns with smart pointers and polymorphism

1 Answer 1

You follow good practices for inheritance

Use of std::shared_ptr

Simplify adding rooms

It's easy to create invalid mazes

Room identifiers

You must log in to answer this question.

Related

Hot Network Questions

Use of `std::shared_ptr`