patterncppMinor
Text-based adventure game using classes and XML
Viewed 0 times
textadventurexmlgameusingbasedclassesand
Problem
I am studying software engineering and we just started learning about object oriented programing. I wanted to be a bit ahead of all others, so I decided to write a little text-adventure using classes.
I would be very glad to know what I can improve and what I can do better. Should I do something differently?
Main.cpp
Game.hpp
Game.cpp
Menu.hpp
```
#ifndef MENU_HPP
#define MENU_HPP
#include
#include
class Menu {
public:
Menu(const st
I would be very glad to know what I can improve and what I can do better. Should I do something differently?
Main.cpp
#include "Game.hpp"
int main(int argc, char const *argv[]) {
Game game = Game();
game.play();
return 0;
}Game.hpp
#ifndef GAME_HPP
#define GAME_HPP
#include "Menu.hpp"
class Game {
public:
Game();
void play();
void road();
std::vector parseXML(std::string file);
};
#endifGame.cpp
#include
#include
#include
#include
#include
#include "Game.hpp"
#include "Menu.hpp"
Game::Game()
{
}
void Game::play()
{
road();
std::cout game = parseXML("game.xml");
auto menu = std::find(game.begin(), game.end(), "mainroad");
while (menu != game.end())
{
menu = std::find(game.begin(), game.end(), menu->takeChoice());
}
}
std::vector Game::parseXML(std::string file)
{
std::vector game;
pugi::xml_document doc;
doc.load_file(file);
pugi::xml_node menus = doc.child("menus");
for (pugi::xml_node menu = menus.child("menu"); menu; menu = menu.next_sibling("menu"))
{
const pugi::char_t* name = menu.attribute("name").value();
const pugi::char_t* prompt = menu.child_value("prompt");
pugi::xml_node choices = menu.child("choices");
std::vector> choices_vector;
for(pugi::xml_node choice = choices.child("choice"); choice; choice = choice.next_sibling("choice"))
{
std::string choice_item = choice.first_child().value();
std::string choice_link = choice.attribute("link").value();
choices_vector.push_back({choice_item, choice_link});
}
game.push_back(Menu(name, prompt, choices_vector));
}
return game;
}Menu.hpp
```
#ifndef MENU_HPP
#define MENU_HPP
#include
#include
class Menu {
public:
Menu(const st
Solution
I like when I see a small main() function! Also the way you check the value from
Here's what I think could be better:
Reference and copy
You are using a
std::find and std::vector
I find the way you use
Naming
A famous quote from Phil Karlton - There are only two hard things in Computer Science: cache invalidation and naming things. It is important that you take some time to name things correctly.
std::cin is nice.Here's what I think could be better:
Reference and copy
You are using a
const std::vector >& in your Menu constructor. This reference avoids a copy when you call the constructor, but as _choices isn't a reference, you still copy your std::vector during the assignment (and that's fortunate because the original variable is destroyed right after the constructor ends, when the current iteration of the for loop ends in Game::parseXML(std::string file)). To avoid this copy, you could use a move assignment.std::find and std::vector
I find the way you use
std::find a bit confusing. It is easy to understand, but doesn't help readability. Appart from that, you are using an std::vector on which you never iterate but only try to access elements by their key (which is currently an instance, but may be their name according to Menu::operator==). You probably want to use an std::unordered_map instead of an std::vector in Game::road.Naming
std::vector game doesn't look like a good name choice. Maybe something like currentLevel would be better. Same for choice and _choices that could be named playerchoice and availableChoices respectively.A famous quote from Phil Karlton - There are only two hard things in Computer Science: cache invalidation and naming things. It is important that you take some time to name things correctly.
Context
StackExchange Code Review Q#148599, answer score: 6
Revisions (0)
No revisions yet.