3
\$\begingroup\$

This is a follow-up from Command-line Tower of Hanoi game -- many thanks to those whose reviwed it.

By request, the project is also available on GitHub. Forks and bug reports are welcomed.

Compiled with g++ 9.4.0. makefile included.

Issues addressed

  1. The disks are further differentiated between even and odd by styling, even disks are solid.
  2. All include guards are now #ifndef/#define.
  3. int and size_t data is now unsigned whereever it makes sense.
  4. Deleted all using namespace std.
  5. Left only main-related code in main().
  6. Replaced entering number of disks at the command-line with asking the player at run-time.
  7. Removed some useless assertions.
  8. Increased modularity of functions/modules.
  9. Game variables wrapped into a struct for easy distribution into functions.
  10. Improved documentation throughout.

Issues not addressed

system() for clearing the screen

As much as I would like to find a safe and portable alternative for system(), I have done some researching, and it seems there is no clean cut solution for this. Workarounds range from using external libraries to abstract low-level differences (want to avoid dependencies like this), to using ANSI escape codes (which I have tried and does not seem to work on Windows). Until I find a better way to do this, the best I can do is passing system() the correct command for the appropriate operating system.

main.cpp

/*
 Author: Jared Thomas
 Date: Tuesday, August 4, 2020
*/
#include "game.h"
#include "screen.h"
int main()
{
 clearScreen();
 game(askNumDisks());
 return 0;
}

game.h

/*
 Author: Jared Thomas
 Date: Thursday, February 2, 2023
 This module contains the core game loop
*/
#ifndef GAME_H
#define GAME_H
/*
 Asks the player for the number of disks they want to play with.
 Blocks program flow until the player enters valid input, where valid input
 is a number between 1 and 8 inclusive.
 Returns the number of disks the player entered.
*/
unsigned askNumDisks();
/*
 Starts the game with an initial number of disks
*/
void game(unsigned initialDisks);
#endif

game.cpp

/*
 Author: Jared Thomas
 Date: Thursday, February 2, 2023
 This module contains the core game loop
*/
#include <vector>
#include <string>
#include <cmath>
#include <iostream>
#include <cassert>
#include "Tower.h"
#include "TowerDrawer.h"
#include "screen.h"
#include "game.h"
#include "game_parser.h"
#include "parse.h"
#include "help.h"
struct GameState {
 GameState(unsigned initialDisks);
 ~GameState();
 std::vector<std::string> tokens;
 bool requestQuit;
 bool gameOver;
 std::vector<Tower> towers;
 TowerDrawer towerDrawer;
 unsigned numDisks; // Total number of game disks
 unsigned moves; // Number of valid moves that the player has made
 std::string status; // Message that prints every frame
 std::string question;
};
GameState::GameState(unsigned initialDisks):
 tokens(),
 requestQuit(false),
 gameOver(false),
 towers(),
 towerDrawer(initialDisks + 3),
 numDisks(initialDisks),
 moves(0),
 status(),
 question()
{
}
GameState::~GameState()
{
}
static unsigned inputState(GameState& gameState);
static void moveState(GameState& gameState);
const unsigned GOAL_TOWER_VECTOR_INDEX = 2;
/*
 Returns the least possible number of moves required to win a game (perfect game).
 This is (2^d - 1), where d is the number of disks.
*/
unsigned leastPossible(unsigned numDisks)
{
 return (1U << numDisks) - 1;
}
/*
 Calculates and returns the game score as the number of moves the player made
 out of the minimum moves required to win
*/
unsigned getScore(unsigned numDisks, unsigned moves)
{
 return (unsigned)(round(100.0 * leastPossible(numDisks) / moves));
}
/*
 Prints the player's game statistics after the game has been won.
 The statistics are the number of moves made, minimum moves required,
 and the final score
*/
void printResults(unsigned numDisks, unsigned moves)
{
 std::cout << "You finished in " << moves << " moves\n";
 std::cout << "Best possible is "\
 << leastPossible(numDisks) << " moves\n";
 std::cout << "Your score: " << getScore(numDisks, moves) << "%";
}
/*
 Clears the screen and draws the towers vector using a tower drawer
*/
void drawTowers(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer)
{
 clearScreen();
 towerDrawer.draw(towers);
}
/*
 Prints the status message after drawing the towers
*/
void printStatus(const std::string& statusMessage)
{
 std::cout << "\n";
 std::cout << statusMessage << "\n";
 std::cout << "\n";
}
void askQuestion(const std::string& question)
{
 std::cout << question;
}
/*
 Returns true if the winning condition has been reached, returns false
 otherwise
*/
bool checkForGameWon(const Tower& goalTower, unsigned totalDisks)
{
 return goalTower.num_disks() == totalDisks;
}
/*
 Puts the towers back in their initial state
*/
void resetTowers(std::vector<Tower>& towers, unsigned totalDisks)
{
 towers.clear();
 towers.push_back(Tower(totalDisks));
 towers.push_back(Tower());
 towers.push_back(Tower());
}
/*
 Puts the entire game back in its initial state; this includes resetting the
 towers and all state variables
*/
void resetGame(GameState& gameState)
{
 resetTowers(gameState.towers, gameState.numDisks);
 gameState.moves = 0;
 gameState.status = "Type \"help\" at any time for instructions. Good luck!";
 gameState.question = "What's your first move? ";
}
/*
 Asks the player if they want to play again. Blocks program flow until the
 player enters valid input.
 Returns true if the player requests to play again, returns false if they
 don't.
*/
bool askPlayAgain()
{
 std::string input;
 do {
 std::cout << "Do you want to play again? (y/n): ";
 input = getRawInput();
 } while(!(input == "y" || input == "n"));
 return input == "y";
}
/*
 Asks the player for the number of disks they want to play with.
 Blocks program flow until the player enters valid input, where valid input
 is a number between 1 and 8 inclusive.
 Returns the number of disks the player entered.
*/
unsigned askNumDisks()
{
 std::string input;
 long numDisks;
 do {
 input.clear();
 numDisks = 0;
 do {
 std::cout << "How many disks do you want to play with? (1=Easiest, 8=Hardest): ";
 input = getRawInput();
 } while(parseLong(input.c_str(), &numDisks) != SUCCESS);
 } while((numDisks < 1) || (numDisks > 8));
 return (unsigned)numDisks;
}
void game(unsigned initialDisks)
{
 GameState gameState(initialDisks);
 resetTowers(gameState.towers, gameState.numDisks);
 std::string rawInput;
 gameState.status = "Type \"help\" at any time for instructions. Good luck!";
 gameState.question = "What's your first move? ";
 while(!(gameState.requestQuit || gameState.gameOver)) {
 drawTowers(gameState.towers, gameState.towerDrawer);
 printStatus(gameState.status);
 askQuestion(gameState.question);
 std::string rawInput = getRawInput();
 gameState.tokens = tokenize(rawInput);
 if(inputState(gameState)) continue;
 moveState(gameState);
 }
}
/*
 Performs input processing and command handling of tokens.
 Returns 0 if the input is a tower move and should be passed on to the move
 parser.
 Returns 1 if the flow should return to the beginning of the game loop.
*/
static unsigned inputState(GameState& gameState)
{
 const unsigned NUM_TUTORIAL_DISKS = 3;
 const unsigned TUTORIAL_ROD_HEIGHT = NUM_TUTORIAL_DISKS + 2;
 std::vector<Tower> tutorialTowers; // Appears on the help text
 TowerDrawer tutorialTowerDrawer(TUTORIAL_ROD_HEIGHT);
 resetTowers(tutorialTowers, NUM_TUTORIAL_DISKS);
 switch(parseInput(gameState.tokens)) {
 case MOVE: return 0;
 case EMPTY_INPUT: return 1;
 case COMMAND:
 {
 switch(parseCommand(gameState.tokens.front())) {
 case REQUEST_QUIT:
 {
 gameState.requestQuit = true;
 return 1;
 }
 case REQUEST_RESET:
 {
 gameState.numDisks = askNumDisks();
 gameState.towerDrawer.set_pole_height(gameState.numDisks + 3);
 resetGame(gameState);
 return 1;
 }
 case REQUEST_HELP:
 {
 showHelpText(tutorialTowers, tutorialTowerDrawer);
 return 1;
 }
 case INVALID_COMMAND:
 {
 gameState.status = "No such command...";
 return 1;
 }
 }
 break; // This only pacifies the compiler; it cannot really be reached.
 }
 case INVALID_INPUT:
 {
 gameState.status = "Huh?";
 return 1;
 }
 }
 assert(0); // Impossible to get here
 return 0;
}
/*
 Takes in tokens from the input stage and attempts to process them as tower
 moves.
*/
static void moveState(GameState& gameState)
{
 TOWER_MOVE towerMove = parseMove(gameState.tokens, gameState.towers);
 switch(towerMove.moveType) {
 case VALID_MOVE:
 {
 doMove(towerMove, gameState.towers);
 gameState.moves++;
 if(checkForGameWon(gameState.towers.at(GOAL_TOWER_VECTOR_INDEX), gameState.numDisks)) {
 drawTowers(gameState.towers, gameState.towerDrawer);
 printStatus("You win!");
 printResults(gameState.numDisks, gameState.moves);
 std::cout << "\n\n";
 gameState.gameOver = !askPlayAgain();
 if(!gameState.gameOver) {
 gameState.numDisks = askNumDisks();
 gameState.towerDrawer.set_pole_height(gameState.numDisks + 3);
 resetGame(gameState);
 }
 return;
 }
 gameState.status = "";
 gameState.question = "What's your next move? ";
 return;
 }
 case DISKLESS_TOWER:
 {
 gameState.status = "Nothing on that tower...";
 return;
 }
 case LARGER_ON_SMALLER:
 {
 gameState.status = "Can't place a larger disk on a smaller disk...";
 return;
 }
 case INVALID_MOVE_SYNTAX:
 {
 gameState.status = "Can't do that...";
 }
 }
}

screen.h

/*
 Author: Jared Thomas
 Date: Sunday, January 29, 2023
 This module provides the platform-dependent clear screen command.
*/
#ifndef SCREEN_H
#define SCREEN_H
/*
 Clears the terminal.
 The actual system call used is platform-dependent.
*/
void clearScreen();
#endif

screen.cpp

/*
 Author: Jared Thomas
 Date: Sunday, January 29, 2023
 This module provides the platform-dependent clear screen command.
*/
#include "screen.h"
#ifdef PLATFORM_LINUX
 #define CLEAR_SCREEN_COMMAND "clear"
#endif
#ifdef PLATFORM_WINDOWS
 #define CLEAR_SCREEN_COMMAND "cls"
#endif
#include <cstdlib>
void clearScreen()
{
 system(CLEAR_SCREEN_COMMAND);
}

parse.h

/*
 Author: Jared Thomas
 Date: Sunday, January 22, 2023
 This module provides general-purpose string processing and parsing utilities.
*/
#ifndef PARSE_H
#define PARSE_H
#include <vector>
#include <string>
/*
 Retrieves input from the console and returns the result as a string.
 Blocks program flow until a newline character is encountered.
*/
std::string getRawInput();
/*
 Splits the string on the space (' ') character, ignoring any leading and
 trailing spaces.
 Returns a vector containing the tokens.
*/
std::vector<std::string> tokenize(const std::string& s);
enum PARSE_LONG_RESULT { INVALID_STRING, UNDERFLOW, OVERFLOW, SUCCESS };
/*
 Attempts to interpret the input string as a base 10 integer. If successful,
 the interpreted value is stored at the location pointed to by result.
 The input should not have leading or trailing spaces.
 If the conversion was successful, SUCCESS is returned.
 If the conversion was successful and the interpreted value would be smaller
 than the minimum value for a (long), UNDERFLOW is returned.
 If the conversion was successful and the interpreted value would be larger
 than the maximum value for a (long), OVERFLOW is returned.
 If the string could not interpreted as a base-10 integer, INVALID_STRING
 is returned.
*/
PARSE_LONG_RESULT parseLong(const char* s, long* result);
#endif

parse.cpp

/*
 Author: Jared Thomas
 Date: Sunday, January 22, 2023
 This module provides general-purpose string processing and parsing utilities.
*/
#include <iostream>
#include <vector>
#include <string>
#include <cstring>
#include <climits>
#include "parse.h"
std::string getRawInput()
{
 std::string input;
 std::getline(std::cin, input);
 return input;
}
std::vector<std::string> tokenize(const std::string& s)
{
 // Create an intermediate string buffer
 const std::size_t BUFFER_LENGTH = s.length() + 1;
 char* buffer = new char[BUFFER_LENGTH];
 memset(buffer, 0, BUFFER_LENGTH);
 // Copy the string into the buffer
 s.copy(buffer, s.length());
 // Tokenize
 std::vector<std::string> result;
 char* token = strtok(buffer, " ");
 while(token) {
 result.push_back(std::string(token));
 token = strtok(nullptr, " ");
 }
 delete[] buffer;
 return result;
}
PARSE_LONG_RESULT parseLong(const char* s, long* result)
{
 const char* afterTheNumber = s + strlen(s);
 char* endPtr = nullptr;
 int previousErrno = errno;
 errno = 0;
 long int longValue = strtol(s, &endPtr, 10);
 if(endPtr != afterTheNumber) {
 errno = previousErrno;
 return INVALID_STRING;
 }
 if(longValue == LONG_MIN && errno == ERANGE) {
 errno = previousErrno;
 return UNDERFLOW;
 }
 if(longValue == LONG_MAX && errno == ERANGE) {
 errno = previousErrno;
 return OVERFLOW;
 }
 errno = previousErrno;
 *result = longValue;
 return SUCCESS;
}

game_parser.h

/*
 Author: Jared Thomas
 Date: Wednesday, February 1, 2023
 This module provides game-specific parsing functions.
*/
#ifndef GAME_PARSER_H
#define GAME_PARSER_H
#include <vector>
#include <string>
#include "Tower.h"
enum INPUT_TYPE { INVALID_INPUT, MOVE, COMMAND, EMPTY_INPUT };
/*
 Low-level parser that returns the type of input the tokens represent.
 If the input is a towers move, MOVE is returned.
 If the input is a game command, COMMAND is returned.
 If the input is an empty string, EMPTY_INPUT is returned.
 If the input was none of the above, INVALID_INPUT is returned.
*/
INPUT_TYPE parseInput(const std::vector<std::string>& tokens);
enum COMMAND_TYPE { REQUEST_QUIT, REQUEST_RESET, INVALID_COMMAND, REQUEST_HELP };
/*
 Mid-level parser that accepts a command string, and returns the requested
 operation
*/
COMMAND_TYPE parseCommand(const std::string& command);
enum MOVE_TYPE { VALID_MOVE, DISKLESS_TOWER, LARGER_ON_SMALLER, INVALID_MOVE_SYNTAX };
struct TOWER_MOVE {
 long int from;
 long int to;
 enum MOVE_TYPE moveType;
};
/*
 High-level parser that accepts a string representing a tower move and a vector
 of towers the moves should be done on.
 Returns a tower move record which contains information about the resulting move.
 The record can be later executed if the move is valid.
 If the semantics of the input are bad (negative or out-of-range tower number, etc.),
 then the returned record will have INVALID_MOVE_SYNTAX as a move type, and the
 from-to fields will be 0.
 If the input string would attempt to take a disk from a rod with no disks,
 then the returned record will have DISKLESS_TOWER as a move type, and the
 from-to fields will be populated with the appropriate tower indices (zero-indexed).
 If the input string would attempt to place a larger disk on top of a smaller disk,
 then the returned record will have LARGER_ON_SMALLER as a move type, and the
 from-to fields will be populated with the appropriate tower indices (zero-indexed).
 If none of the above cases occurred,
 then the returned record will have VALID_MOVE as a move type, and the
 from-to fields will be populated with the appropriate tower indices (zero-indexed).
*/
TOWER_MOVE parseMove(const std::vector<std::string>& tokens, const std::vector<Tower>& towers);
/*
 Executes a tower move record on the input vector of towers.
 The move type must not be INVALID_MOVE_SYNTAX or DISKLESS_TOWER.
 Move type LARGER_ON_SMALLER is allowed, but violates the traditional rules
 of the game.
*/
void doMove(TOWER_MOVE move, std::vector<Tower>& towers);
#endif

game_parser.cpp

/*
 Author: Jared Thomas
 Date: Wednesday, February 1, 2023
 This module provides game-specific parsing functions.
*/
#include <vector>
#include <string>
#include "game_parser.h"
#include "parse.h"
#include "Tower.h"
INPUT_TYPE parseInput(const std::vector<std::string>& input)
{
 // If there are two tokens, then treat the input as valid syntax for a move.
 // XXX XXXX
 // If there is only one token in the input, then treat it as a command.
 // XXXXXXX
 // If there are no tokens, then this is empty input.
 // If there are more than two tokens, then the input is invalid.
 // XXX XXXX XX
 switch(input.size()) {
 case 0: return EMPTY_INPUT;
 case 1: return COMMAND;
 case 2: return MOVE;
 default: return INVALID_INPUT;
 }
}
COMMAND_TYPE parseCommand(const std::string& command)
{
 if(command == "quit") return REQUEST_QUIT;
 if(command == "reset") return REQUEST_RESET;
 if(command == "help") return REQUEST_HELP;
 return INVALID_COMMAND;
}
TOWER_MOVE parseMove(const std::vector<std::string>& tokens, const std::vector<Tower>& towers)
{
 long int from, to;
 PARSE_LONG_RESULT fromResult = parseLong(tokens.at(0).c_str(), &from);
 PARSE_LONG_RESULT toResult = parseLong(tokens.at(1).c_str(), &to);
 // Return this when processing the move would result in a fatal error.
 const TOWER_MOVE PROBLEM_MOVE = { 0, 0, INVALID_MOVE_SYNTAX };
 if(!(fromResult == SUCCESS && toResult == SUCCESS)) {
 return PROBLEM_MOVE;
 }
 if((from < 1) || (from > 3)) {
 return PROBLEM_MOVE;
 }
 if((to < 1) || (to > 3)) {
 return PROBLEM_MOVE;
 }
 from--;
 to--;
 const Tower& towerFrom = towers.at(from);
 const Tower& towerTo = towers.at(to);
 if(towerFrom.is_diskless()) {
 TOWER_MOVE result = { from, to, DISKLESS_TOWER };
 return result;
 }
 if(!towerTo.is_diskless() &&
 (towerFrom.size_of_top() > towerTo.size_of_top())) {
 TOWER_MOVE result = { from, to, LARGER_ON_SMALLER };
 return result;
 }
 TOWER_MOVE result = { from, to, VALID_MOVE };
 return result;
}
void doMove(const TOWER_MOVE move, std::vector<Tower>& towers)
{
 Tower& towerFrom = towers.at(move.from);
 Tower& towerTo = towers.at(move.to);
 towerFrom.top_to_top(towerTo);
}

help.h

/*
 Author: Jared Thomas
 Date: Monday, January 23, 2023
 This module provides the help command handler.
*/
#ifndef HELP_H
#define HELP_H
#include <vector>
#include "Tower.h"
#include "TowerDrawer.h"
/*
 Clears the screen and starts the help texts which come in two screens.
 The passed in towers will be shown as a demonstration in the explanation.
*/
void showHelpText(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer);
/*
 Shows part 1/2 of the help text which is the game objective, rules, and
 inputs explanation
*/
void showExpanation(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer);
/*
 Shows part 2/2 of the help text which is the list of game commands
*/
void showCommandsHelp();
#endif

help.cpp

/*
 Author: Jared Thomas
 Date: Monday, January 23, 2023
 This module provides the help command handler.
*/
#include "help.h"
#include <vector>
#include <cstdlib>
#include <iostream>
#include "Tower.h"
#include "TowerDrawer.h"
#include "screen.h"
#include "parse.h"
void showHelpText(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer)
{
 clearScreen();
 showExpanation(towers, towerDrawer);
 std::cout << "\n\n";
 std::cout << "Press \"Enter\" for the list of commands...";
 getRawInput();
 clearScreen();
 showCommandsHelp();
 std::cout << "\n\n";
 std::cout << "Press \"Enter\" to go back to the game...";
 getRawInput();
}
void showExpanation(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer)
{
 std::cout << "Towers, an adaptation of the game \"Tower of Hanoi\"\n";
 std::cout << "Nexus Game Studios, 2023\n";
 std::cout << "Programming, Jared Thomas\n";
 std::cout << "\n";
 std::cout << "The goal of Towers is to move all the disks from the leftmost rod to the rightmost rod.\n";
 std::cout << "Sounds easy, right? But not so fast!\n";
 std::cout << "You can only move the topmost disk from any tower.\n";
 std::cout << "On top of that, you can't put a larger disk on top of a smaller one!\n";
 towerDrawer.draw(towers);
 std::cout << "\n";
 std::cout << "To move a disk from one rod to another, type the rod number you want to\n";
 std::cout << "move from, then the rod number to move to, separated by a space. Like this:\n";
 std::cout << "\n";
 std::cout << "1 2\n";
 std::cout << "\n";
 std::cout << "This would move the topmost disk from the left rod to the middle rod.\n";
 std::cout << "If you can move all the disks to the rightmost rod, you win!";
}
void showCommandsHelp()
{
 std::cout << "Commands\n";
 std::cout << "\n";
 std::cout << "quit Quit the game\n";
 std::cout << "help Show the game explanation, rules, and commands\n";
 std::cout << "reset Start the game over again";
}

Disk.h

#ifndef DISK_H
#define DISK_H
struct Disk {
 /*
 Constructs a disk with a size
 size > 0
 */
 Disk(unsigned size);
 /*
 Draws the disk to standard output
 */
 void draw() const;
 /*
 Returns the size of the disk
 */
 unsigned size() const;
private:
 unsigned size_;
};
void draw_solid_style(Disk);
void draw_slash_bracket_style(Disk);
#endif

Disk.cpp

#include "Disk.h"
#include <cassert>
#include <iostream>
Disk::Disk(unsigned size): size_(size)
{
 assert(size_ > 0);
}
void Disk::draw() const
{
 for(unsigned i = 0; i < (2 * size() + 1); i++) {
 std::cout << '+';
 }
}
unsigned Disk::size() const { return size_; }
void draw_solid_style(const Disk d)
{
 std::cout << '['; // Left edge of the disk
 unsigned j = 2 * d.size() + 1;
 if(d.size() >= 10 && d.size() <= 99) j--;
 for(unsigned i = 0; i < j; i++) {
 const unsigned DISK_CENTER = (2 * d.size() + 1) / 2;
 if(i == DISK_CENTER) std::cout << d.size();
 else std::cout << ' ';
 }
 std::cout << ']'; // Right edge of the disk
}
void draw_slash_bracket_style(const Disk d)
{
 std::cout << '['; // Left edge of the disk
 unsigned j = 2 * d.size() + 1;
 if(d.size() >= 10 && d.size() <= 99) j--;
 for(unsigned i = 0; i < j; i++) {
 const unsigned DISK_CENTER = (2 * d.size() + 1) / 2;
 if(i == DISK_CENTER) std::cout << d.size();
 else if(i == (DISK_CENTER - 1)) std::cout << ' ';
 else if(i == (DISK_CENTER + 1)) std::cout << ' ';
 else std::cout << '/';
 }
 std::cout << ']'; // Right edge of the disk
}

Tower.h

#ifndef TOWER_H
#define TOWER_H
#include "Disk.h"
#include <vector>
#include <cstdlib>
class Tower {
public:
 /*
 Constructs a tower with no disks (diskless)
 */
 Tower();
 /*
 Constructs a tower with an initial number of disks.
 The disks will be stacked in increasing order from top to bottom.
 */
 Tower(unsigned num_disks);
 /*
 Returns the number of disks on the tower
 */
 unsigned num_disks() const;
 /*
 Returns the size of the topmost disk on the tower
 */
 unsigned size_of_top() const;
 /*
 Returns the size of the largest disk on the tower
 */
 unsigned size_of_largest_disk() const;
 /*
 Returns the size of a disk at a specific location on the tower.
 0 is the bottom of the tower.
 */
 unsigned size_of_disk_at(unsigned index) const;
 /*
 Returns true if the tower has no disks on it, returns false otherwise
 */
 bool is_diskless() const;
 /*
 */
 bool are_strictly_decreasing() const;
 /*
 Returns a reference to a specific disk on the tower
 */
 const Disk& disk_at(unsigned index) const;
 /*
 Moves the topmost disk from this tower to another tower
 */
 void top_to_top(Tower& dest_tower);
 /*
 */
 bool compare(const Tower&) const;
private:
 std::vector<Disk> disks_;
};
/*
 Returns the number of disks on the tower with the most disks in the vector
*/
unsigned highestTower(const std::vector<Tower>& towers);
#endif

Tower.cpp

#include "Tower.h"
#include "Disk.h"
#include <cassert>
#include <climits>
#include <iostream>
Tower::Tower()
{
}
Tower::Tower(unsigned num_disks)
{
 while(num_disks > 0) {
 disks_.push_back(Disk(num_disks));
 num_disks--;
 }
}
unsigned Tower::num_disks() const
{
 // The cast is okay here since the number of disks on the tower is
 // guaranteed to be [0, UINT_MAX]
 return (unsigned)(disks_.size());
}
unsigned Tower::size_of_top() const
{
 assert(!is_diskless());
 return disks_.back().size();
}
unsigned Tower::size_of_largest_disk() const
{
 assert(!is_diskless());
 unsigned largest = size_of_disk_at(0);
 for(unsigned i = 0; i < num_disks(); i++) {
 if(size_of_disk_at(i) > largest) largest = size_of_disk_at(i);
 }
 return largest;
}
unsigned Tower::size_of_disk_at(unsigned index) const
{
 assert(!is_diskless());
 return disks_.at(index).size();
}
bool Tower::is_diskless() const { return num_disks() == 0; }
bool Tower::are_strictly_decreasing() const
{
 assert(!is_diskless());
 unsigned expected = size_of_disk_at(0);
 for(unsigned i = 0; i < num_disks(); i++) {
 if(size_of_disk_at(i) != expected) return false;
 expected--;
 }
 return true;
}
const Disk& Tower::disk_at(unsigned index) const
{
 assert(!is_diskless());
 return disks_.at(index);
}
void Tower::top_to_top(Tower& dest_tower)
{
 assert(dest_tower.num_disks() < UINT_MAX);
 assert(!is_diskless());
 Disk diskToMove = disks_.back();
 disks_.pop_back();
 dest_tower.disks_.push_back(diskToMove);
}
bool Tower::compare(const Tower& T) const
{
 if(T.num_disks() != num_disks()) return false;
 if(T.is_diskless() && is_diskless()) return true;
 for(unsigned i = 0; i < num_disks(); i++) {
 if(T.disk_at(i).size() != disk_at(i).size()) return false;
 }
 return true;
}
unsigned highestTower(const std::vector<Tower>& towers)
{
 assert(!towers.empty());
 unsigned highest = 0;
 for(size_t i = 0; i < towers.size(); i++) {
 if(towers.at(i).num_disks() > highest) {
 highest = towers.at(i).num_disks();
 }
 }
 return highest;
}

TowerDrawer.h

#ifndef TOWER_DRAWER
#define TOWER_DRAWER
#include <cstddef>
#include <vector>
class Tower;
class TowerDrawer {
public:
 TowerDrawer(unsigned pole_height);
 unsigned pole_height() const;
 size_t draw(const Tower&) const;
 size_t draw(const std::vector<Tower>&) const;
 void set_pole_height(unsigned pole_height);
private:
 void draw_spaces(unsigned num_spaces = 1) const;
 unsigned num_slashes(unsigned disk_size) const;
 unsigned num_chars(unsigned disk_size) const;
 unsigned center_of(unsigned disk_size) const;
 void draw_disk_row(unsigned disk_index, const Tower&) const;
 void draw_rod_row(const Tower&) const;
 void draw_rod_top(const Tower&) const;
 void draw_tower_row(unsigned row, const Tower&) const;
 unsigned pole_height_;
};
#endif

TowerDrawer.cpp

#include "TowerDrawer.h"
#include "Tower.h"
#include <cassert>
#include <iostream>
#include <climits>
TowerDrawer::TowerDrawer(unsigned pole_height): pole_height_(pole_height)
{}
unsigned TowerDrawer::pole_height() const { return pole_height_; }
size_t TowerDrawer::draw(const Tower& T) const
{
 std::vector<Tower> tempTowerVector;
 tempTowerVector.push_back(T);
 return draw(tempTowerVector);
}
size_t TowerDrawer::draw(const std::vector<Tower>& towers) const
{
 if(towers.empty()) return 0;
 assert(pole_height_ > highestTower(towers));
 for(unsigned i = 0; i <= pole_height_; i++) {
 for(size_t t = 0; t < towers.size(); t++) {
 draw_tower_row(pole_height_ - i, towers.at(t));
 draw_spaces(12);
 }
 std::cout << "\n";
 }
 return towers.size();
}
void TowerDrawer::set_pole_height(unsigned pole_height)
{
 pole_height_ = pole_height;
}
void TowerDrawer::draw_spaces(unsigned n) const
{
 for(; n > 0; n--) std::cout << " ";
}
unsigned TowerDrawer::num_slashes(const unsigned disk_size) const
{
 return disk_size * 2 + 1;
}
unsigned TowerDrawer::num_chars(const unsigned disk_size) const
{
 return 2 + num_slashes(disk_size);
}
unsigned TowerDrawer::center_of(const unsigned disk_size) const
{
 return (num_chars(disk_size) - 1) / 2;
}
void TowerDrawer::draw_disk_row(const unsigned disk_index, const Tower& t) const
{
 draw_spaces(center_of(t.size_of_largest_disk()) - center_of(t.size_of_disk_at(disk_index)));
 if(!(t.size_of_disk_at(disk_index) & 1)) draw_slash_bracket_style(t.disk_at(disk_index));
 else draw_solid_style(t.disk_at(disk_index));
 draw_spaces(center_of(t.size_of_largest_disk()) - center_of(t.size_of_disk_at(disk_index)));
}
void TowerDrawer::draw_rod_row(const Tower& t) const
{
 if(t.is_diskless()) {
 draw_spaces();
 std::cout << "|_|";
 draw_spaces();
 return;
 }
 draw_spaces(center_of(t.size_of_largest_disk()) - 1);
 std::cout << "|_|";
 draw_spaces(center_of(t.size_of_largest_disk()) - 1);
}
void TowerDrawer::draw_rod_top(const Tower& t) const
{
 if(t.is_diskless()) {
 draw_spaces(2);
 std::cout << "_";
 draw_spaces(2);
 return;
 }
 draw_spaces(center_of(t.size_of_largest_disk()));
 std::cout << "_";
 draw_spaces(center_of(t.size_of_largest_disk()));
}
// todo: draws funnily when tower.num_disks() == tower.height()
void TowerDrawer::draw_tower_row(unsigned row, const Tower& tower) const
{
 assert(row <= pole_height_);
 if(row == pole_height_) draw_rod_top(tower);
 else if(row >= tower.num_disks()) draw_rod_row(tower);
 else draw_disk_row(row, tower);
}

makefile

all: linux-debug win-64-debug
linux-debug: towers-de
towers-de: main.cpp game.cpp screen.cpp parse.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp game_parser.cpp
 g++ -std=c++17 -Wall -Wextra -Wpedantic -Wconversion -D PLATFORM_LINUX main.cpp game.cpp screen.cpp parse.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp game_parser.cpp -o towers-de -g
win-64-debug: towers-de-win-x86-64.exe
towers-de-win-x86-64.exe: main.cpp game.cpp screen.cpp parse.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp game_parser.cpp
 x86_64-w64-mingw32-g++ -std=c++17 -Wall -Wextra -Wpedantic -Wconversion -D PLATFORM_WINDOWS main.cpp game.cpp screen.cpp parse.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp game_parser.cpp -o towers-de-win-x86-64 -g
clean:
 rm -f towers-de
 rm -f towers-de-win-x86-64.exe
asked Feb 5, 2023 at 15:23
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

It's great to see all the improvements you've incorporated into your game!

Clearing the screen

As much as I would like to find a safe and portable alternative for system(), I have done some researching, and it seems there is no clean cut solution for this. ... Until I find a better way to do this, the best I can do is passing system() the correct command for the appropriate operating system.

You did at least address the issue that your previous solution did not work on Windows. But all the drawbacks of system() remain, in particular it is very inefficient and it might not actually do what you want if the user has a different shell environment where clear or cls do something unexpected. Also, even if there is no "clean cut solution", you can do better than this.

Another issue you introduced is that you used a macro to define CLEAR_SCREEN_COMMAND, however apart from the include guards, you very rarely need to create macros in C++. You can instead define a regular constant:

#ifdef PLATFORM_WINDOWS
static const char CLEAR_SCREEN_COMMAND[] = "cls";
#else
static const char CLEAR_SCREEN_COMMAND[] = "clear";
#endif

Have a look at this StackOverflow question about clearing the screen in C++. There are various solutions prevented for both UNIX and Windows, many of them not needing system(). I would structure your code like so:

void clearScreen()
{
#ifdef PLATFORM_WINDOWS
 // Windows-specific code that doesn't use system() here
 ...
#else
 std::cout << "033円[2J";
#endif
}

Use the Doxygen format to document your code

  1. Improved documentation throughout.

That's great, the documentation you wrote looks very good. Even better would be to make some small modifications so it conforms to the Doxygen format. This then allows the Doxygen tools to generate searchable HTML and PDF documents for your source code, among other things. It also can check that you documented all function.

Doxygen also has standards for how to document function parameters and return values. Consider adding that as well. You can also document member variables.

Avoid C functions if there are better C++ functions

This is another issue you did not address since the previous version.

Some of your code calls C functions when there are much better ways to do the same thing in C++. For example, parseLong() calls strtol() and deals with all the issues that that function has, but you could have used std::stol() instead, or possibly std::from_chars().

There are many ways to tokenize a string in C++ without having to resort to strtok(), and without needing any manual memory allocation.

Return an enum from inputState()

The function inputState() returns an integer, and you have to look up the documentation for the function to check what value means what. While it is great that there is documentation, it would be even better if you didn't have to check that. You are already using enums in many other places, including the return values for parseInput() and parseCommand(), you can do something similar here.

Complexity

@pacmaninbw mentioned complexity in the review of the previous version of your game. I think you already have done a good job keeping complexity down, at the point of even having very trivial functions like askQuestion(). However, there is still an area where complexity is higher than desired, and that is GameState. It has member variables dealing with the towers themselves, as well as variables dealing with the user interface.

Just like you can split up functions, you can split up classes as well. Consider:

struct TowerState {
 ...
};
struct UIState {
 ...
};
struct GameState {
 TowerState towerState;
 UIState uiState;
};

Functions that only need to access the state of the towers can be passed gameState.towerState, functions that need only to deal with the UI can be passed gameState.uiState, and only functions that need both can be given access to gameState as a whole. This simplifies the structs and makes it easier to keep things separate.

Don't add empty destructors

You don't have to write an empty destructor, C++ will generate a default destructor if you omit it.

Consider using default member initialization

Instead of explicitly initializing everything in the initializer list of the constructor, consider providing default values when declaring the member variables, for example:

struct GameState {
 GameState(unsigned initialDisks);
 std::vector<std::string> tokens;
 bool requestQuit = false;
 bool gameOver = false;
 std::vector<Tower> towers;
 TowerDrawer towerDrawer;
 unsigned numDisks;
 unsigned moves = 0;
 std::string status;
 std::string question;
};
GameState::GameState(unsigned initialDisks):
 towerDrawer(initialDisks + 3),
 numDisks(initialDisks)
{
}

The advantage is that it's easier to add a new member variable and provide a default value right there, with less chance of forgetting to add an initializer to the constructor as well. And if you have multiple constructors, it can save a lot of code duplication.

answered Feb 5, 2023 at 19:18
\$\endgroup\$
1
  • \$\begingroup\$ Very good, I may have missed some things. \$\endgroup\$ Commented Feb 5, 2023 at 19:27
3
\$\begingroup\$

General Observations

The code is definitely improving! A lot of the issues found in the original question have been fixed.

Thank you for creating the GitHub repository it makes looking at the code much easier!

Both here and on GitHub you use the ID Mode77, but the code contains comments with your real name. Which do you want out in the public domain?

You are already close to this but you might want to look into design patterns such as Model View View Model (MVVM) and Model View Controller (MVC). The Tower class would be your model, the TowerDrawer class would be the view. This might eliminate the files screen.h and screen.c. It might also let you use a graphic interface rather than text with a minimum amount of effort.

I have run out of time and I will leave room for other reviews.

Portability

The code is portable as long as the make file is used, it is dependent on symbols in the makefile. Windows has it's own symbols that can be used itstead. I tried compiling with Visual Studio 2022 and this code doesn't compile in screen.cpp:

#ifdef PLATFORM_LINUX
 #define CLEAR_SCREEN_COMMAND "clear"
#endif
#ifdef PLATFORM_WINDOWS
 #define CLEAR_SCREEN_COMMAND "cls"
#endif
#include <cstdlib>
void clearScreen()
{
 system(CLEAR_SCREEN_COMMAND);
}

However, this code does compile and run:

#ifdef __linux__
 #define CLEAR_SCREEN_COMMAND "clear"
#endif
#ifdef _WIN32
#define CLEAR_SCREEN_COMMAND "cls"
#endif
#include <cstdlib>
void clearScreen()
{
 system(CLEAR_SCREEN_COMMAND);
}

For a list of macros that work on different systems and additional references please see this stack overflow question.

Since you are already using #ifdefs in the code what you can do rather than using the system() function is use the alternate methods you researched.

Format of Header Files

Group similar types of declarations together, such as enums with enums and function prototypes with function prototypes. It is possible that some or all of the comments in parse.h should be in parse.cpp

I'm not sure how important this comment is, Returns a vector containing the tokens. it should be obvious from the code.

#ifndef PARSE_H
#define PARSE_H
#include <vector>
#include <string>
enum PARSE_LONG_RESULT { INVALID_STRING, MYUNDERFLOW, MYOVERFLOW, SUCCESS };
/*
 Attempts to interpret the input string as a base 10 integer. If successful,
 the interpreted value is stored at the location pointed to by result.
 The input should not have leading or trailing spaces.
 If the conversion was successful, SUCCESS is returned.
 If the conversion was successful and the interpreted value would be smaller
 than the minimum value for a (long), UNDERFLOW is returned.
 If the conversion was successful and the interpreted value would be larger
 than the maximum value for a (long), OVERFLOW is returned.
 If the string could not interpreted as a base-10 integer, INVALID_STRING
 is returned.
*/
PARSE_LONG_RESULT parseLong(const char* s, long* result);
/*
 Splits the string on the space (' ') character, ignoring any leading and
 trailing spaces.
 */
std::vector<std::string> tokenize(const std::string& s);
/*
 Retrieves input from the console and returns the result as a string.
 Blocks program flow until a newline character is encountered.
 */
std::string getRawInput();
#endif

Write Self Documenting Code

Write self documenting code as much as possible, the problem with comments is that they need to be maintained as much as the code does. The Tower.cpp file and the TowerDrawer.cpp files are more in line with self documenting code than most of the other files. Comments should be used to explain design decisions and why the code is written the way it is.

Comments such as :

 /*
 Constructs a tower with an initial number of disks.
 The disks will be stacked in increasing order from top to bottom.
 */

are not that useful, perhaps the second line is useful, but Constructs a tower with an initial number of disks. is pretty obvious.

Tokenizing the Input

The code in the function std::vector<std::string> tokenize(const std::string& s) is currently more like the C programming language than idiomatic C++. There are multiple ways to do this in the C++ programming language including using regex. This is deeply discussed in the stack overflow question How do i tokenize a string in C++. I suggest that the third answer would prove quite useful.

Game

In the game.cpp file there are many functions that should be declared static that are not declared static, some examples are:

  • unsigned leastPossible(unsigned numDisks)
  • unsigned getScore(unsigned numDisks, unsigned moves)
  • void printResults(unsigned numDisks, unsigned moves)
  • void drawTowers(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer)
  • void printStatus(const std::string& statusMessage)
  • void askQuestion(const std::string& question)
  • bool checkForGameWon(const Tower& goalTower, unsigned totalDisks)

Basically all the functions except for the 2 global entry points in game.h.

It is possible that the game should be a class where the public interfaces would be a constructor game(unsigned initialDisks) and a method called run(). Then all the other functions could be private member methods.

This is much more procedural than it could be, it could definitely be converted to object oriented programming. OOP allows you more flexability and the code is easier to reuse.

answered Feb 5, 2023 at 19:25
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.