5
\$\begingroup\$

This code has been revised. See (Rev. 2) Command-line Tower of Hanoi game

Compiled with g++ 9.4.0. makefile included. Any criticism welcome.

main.cpp

/*
    Author: Jared Thomas
    Date:   Tuesday, August 4, 2020
*/

#include <iostream>
#include <string>
#include <vector>
#include <cstdlib>
#include <cstring>
#include <cmath>

#include "parse.h"
#include "move_parser.h"
#include "help.h"
#include "Tower.h"
#include "TowerDrawer.h"



unsigned leastPossible(size_t num_disks)
{
    return pow(2, num_disks) - 1;
}


double getScore(size_t num_disks, unsigned moves)
{
    return round(100.0 * leastPossible(num_disks) / moves);
}


void printResults(size_t num_disks, unsigned moves)
{
    std::cout << "You finished in " << moves << " moves\n";

    std::cout << "Best possible is "\
        << leastPossible(num_disks) << " moves\n";

    std::cout << "Your score: " << getScore(num_disks, moves) << "%";
}


void drawTowers(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer)
{
    system("clear");
    towerDrawer.draw(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;
}


enum INPUT_TYPE { INVALID_INPUT, MOVE, COMMAND, EMPTY_INPUT };

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;
    }
}


enum COMMAND_TYPE { REQUEST_QUIT, REQUEST_RESET, INVALID_COMMAND, REQUEST_HELP };

COMMAND_TYPE parseCommand(const std::vector<std::string>& input)
{
    std::string command(input.front());
    if(command == "quit") return REQUEST_QUIT;
    if(command == "reset") return REQUEST_RESET;
    if(command == "help") return REQUEST_HELP;
    return INVALID_COMMAND;
}


bool checkForGameWon(const Tower& goalTower, int totalDisks)
{
    return goalTower.num_disks() == totalDisks;
}


void resetTowers(std::vector<Tower>& towers, int totalDisks)
{
    towers.clear();
    towers.push_back(Tower(totalDisks));
    towers.push_back(Tower());
    towers.push_back(Tower());
}


void resetGame(std::vector<Tower>& towers, int numDisks, int& moves, std::string& statusMessage, std::string& prompt)
{
    resetTowers(towers, numDisks);
    moves = 0;
    statusMessage = "Type \"help\" at any time for instructions. Good luck!";
    prompt = "What's your first move? ";
}


bool askPlayAgain()
{
    char inputChar;
    do {
        std::cout << "Do you want to play again? (y/n): ";
        std::cin >> inputChar;
    } while(!(inputChar == 'y' || inputChar == 'n'));
    return inputChar == 'y';
}


int main(int argc, char* argv[])
{
    const int NUM_DISKS = (argc == 2) ? std::stoi(argv[1]) : 3;
    const int NUM_TUTORIAL_DISKS = 3;
    const int TUTORIAL_ROD_HEIGHT = NUM_TUTORIAL_DISKS + 2;
    const int GOAL_TOWER_VECTOR_INDEX = 2;

    std::vector<Tower> towers;          // Actual game rods
    TowerDrawer tower_drawer(NUM_DISKS + 3);
    resetTowers(towers, NUM_DISKS);

    std::vector<Tower> tutorialTowers;  // Appears on the help text
    TowerDrawer tutorialTowerDrawer(TUTORIAL_ROD_HEIGHT);
    resetTowers(tutorialTowers, NUM_TUTORIAL_DISKS);

    int moves = 0;
    bool requestQuit = false;
    bool gameOver = false;
    std::string status, question, rawInput;
    status = "Type \"help\" at any time for instructions. Good luck!";
    question = "What's your first move? ";
    while(!(requestQuit || gameOver)) {
        drawTowers(towers, tower_drawer);
        printStatus(status);
        askQuestion(question);
        std::string rawInput = getRawInput();
        std::vector<std::string> tokens = tokenize(rawInput);

        switch(parseInput(tokens)) {
        case MOVE: break;
        case EMPTY_INPUT: continue;
        case COMMAND:
            {
                switch(parseCommand(tokens)) {
                case REQUEST_QUIT:
                    {
                        requestQuit = true;
                        continue;
                    }
                case REQUEST_RESET:
                    {
                        resetGame(towers, NUM_DISKS, moves, status, question);
                        continue;
                    }
                case REQUEST_HELP:
                    {
                        system("clear");
                        showHelpText(tutorialTowers, tutorialTowerDrawer);
                        std::cout << "\n\n";
                        std::cout << "Press \"Enter\" for the list of commands...";
                        getRawInput();
                        system("clear");
                        showCommandsHelp();
                        std::cout << "\n\n";
                        std::cout << "Press \"Enter\" to go back to the game...";
                        getRawInput();
                        continue;
                    }
                case INVALID_COMMAND:
                    {
                        status = "No such command...";
                        continue;
                    }
                }
            }
        case INVALID_INPUT:
            {
                status = "Huh?";
                continue;
            }
        }

        TOWER_MOVE towerMove = parseMove(tokens, towers);
        switch(towerMove.moveType) {
        case VALID_MOVE:
            {
                doMove(towerMove, towers);
                moves++;
                if(checkForGameWon(towers.at(GOAL_TOWER_VECTOR_INDEX), NUM_DISKS)) {
                    drawTowers(towers, tower_drawer);
                    printStatus("You win!");
                    printResults(NUM_DISKS, moves);
                    std::cout << "\n\n";
                    gameOver = !askPlayAgain();
                    if(!gameOver) {
                        resetGame(towers, NUM_DISKS, moves, status, question);
                    }
                    continue;
                }
                status = "";
                question = "What's your next move? ";
                break;
            }
        case DISKLESS_TOWER:
            {
                status = "Nothing on that tower...";
                break;
            }
        case LARGER_ON_SMALLER:
            {
                status = "Can't place a larger disk on a smaller disk...";
                break;
            }
        case INVALID_MOVE_SYNTAX:
            {
                status = "Can't do that...";
                break;
            }
        }
    }

    return 0;
}

parse.h

/*
    Author: Jared Thomas
    Date:   Sunday, January 22, 2023

    This module provides 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
std::string getRawInput();

// Splits the string on the space (' ') character. Ignores leading 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 };

// The input will not have leading or trailing spaces.
PARSE_LONG_RESULT parseLong(const char* s, long* result);

#endif

parse.cpp

/*
    Author: Jared Thomas
    Date:   Sunday, January 22, 2023

    This module provides string processing and parsing utilities.
*/

#include <iostream>
#include <vector>
#include <string>
#include <cstring>
#include <climits>
#include "parse.h"

// Retrieves input from the console and returns the result as a string
std::string getRawInput()
{
    std::string input;
    std::getline(std::cin, input);
    return input;
}

// Splits the string on the space (' ') character. Ignores leading spaces.
// Returns a vector containing the tokens.
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;
}

// The input will not have leading or trailing spaces.
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;
}

move_parser.h

/*
    Author: Jared Thomas
    Date:   Sunday, January 22, 2023

    This module provides higher-order parsing for Towers moves.
*/

#ifndef MOVE_PARSER_H
#define MOVE_PARSER_H

#include <vector>
#include <string>

#include "Tower.h"

/*
    DISKLESS_TOWER - Attempt to take a disk from a rod with no disks.
    LARGER_ON_SMALLER - Attempt to place a larger disk on a smaller disk.
    INVALID_MOVE_SYNTAX - The input could not be processed due to one or more
    of the following reasons:
        1. Some input couldn't be converted to a numerical type
        2. Some numerical identifier would be out of range
*/
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;
};

/*
    Parses tokens and produces a tower move record.
*/
TOWER_MOVE parseMove(const std::vector<std::string>& tokens, const std::vector<Tower>& towers);

/*
    Executes the towers move.

    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

move_parser.cpp

/*
    Author: Jared Thomas
    Date:   Sunday, January 22, 2023

    This module provides higher-order parsing for Towers moves.
*/

#include <vector>
#include <string>

#include "move_parser.h"
#include "parse.h"
#include "Tower.h"

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 terminal and prints the help text explaining the rules and goal
// of the game.
//
// The passed in towers will be shown as a demonstration in the help text.
void showHelpText(const std::vector<Tower>& towers, const TowerDrawer& towerDrawer);

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"

// Clears the terminal and prints the help text explaining the rules and goal
// of the game.
//
// The passed in towers will be shown as a demonstration in the help text.
void showHelpText(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!";
}

// Prints the commands help text.
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

#pragma once

struct Disk {
    // size > 0
    Disk(const int size);

    void draw() const;
    int size() const;

private:
    int size_;
};

void draw_solid_style(const Disk);
void draw_slash_bracket_style(const Disk);

Disk.cpp

#include "Disk.h"
#include <cassert>
#include <iostream>


using namespace std;

Disk::Disk(const int size): size_(size)
{
    assert(size_ > 0);
}

void Disk::draw() const
{
    for(int i = 0; i < (2 * size() + 1); i++) {
        cout << '+';
    }
}

int Disk::size() const { return size_; }


void draw_solid_style(const Disk d)
{
    cout << '[';
    int j = 2 * d.size() + 1;
    if(d.size() >= 10 && d.size() <= 99) j--;
    for(int i = 0; i < j; i++) {
        if(i == (2 * d.size() + 1) / 2) cout << d.size();
        else cout << ' ';
    }
    cout << ']';
}


void draw_slash_bracket_style(const Disk d)
{
    cout << '[';
    int j = 2 * d.size() + 1;
    if(d.size() >= 10 && d.size() <= 99) j--;
    for(int i = 0; i < j; i++) {
        if(i == (2 * d.size() + 1) / 2) cout << d.size();
        else if(i == ((2 * d.size() + 1) / 2) - 1) cout << ' ';
        else if(i == ((2 * d.size() + 1) / 2) + 1) cout << ' ';
        else cout << '/';
    }
    cout << ']';
}

Tower.h

#pragma once

#include "Disk.h"
#include <vector>

class Tower {
public:
    Tower();
    Tower(size_t num_disks);

    size_t num_disks() const;
    int size_of_top() const;
    int size_of_largest_disk() const;
    int size_of_disk_at(size_t place) const;
    bool is_diskless() const;
    bool are_strictly_decreasing() const;
    const Disk& disk_at(size_t index) const;
    void top_to_top(Tower& dest_tower);
    bool compare(const Tower&) const;

private:
    std::vector<Disk> disks_;
};

size_t highestTower(const std::vector<Tower>& towers);

Tower.cpp

#include "Tower.h"
#include "Disk.h"
#include <cassert>
#include <iostream>

using namespace std;



Tower::Tower()
{

}


Tower::Tower(size_t num_disks)
{
    assert(num_disks >= 0);
    for(int i = num_disks; i > 0; i--) {
        disks_.push_back(Disk(i));
    }
}


size_t Tower::num_disks() const { return disks_.size(); }


int Tower::size_of_top() const
{
    assert(!is_diskless());
    return disks_.back().size();
}


int Tower::size_of_largest_disk() const
{
    assert(!is_diskless());
    int largest = 0;
    for(size_t u = 0; u < num_disks(); u++) {
        if(size_of_disk_at(u) > largest) largest = size_of_disk_at(u);
    }
    return largest;
}


int Tower::size_of_disk_at(size_t place) const
{
    assert(!is_diskless());
    return disks_.at(place).size();
}


bool Tower::is_diskless() const { return num_disks() == 0; }


bool Tower::are_strictly_decreasing() const
{
    assert(!is_diskless());
    size_t expected = num_disks();
    for(size_t j = 0; j < num_disks(); j++) {
        if(size_of_disk_at(j) != expected) return false;
        expected--;
    }
    return true;
}


const Disk& Tower::disk_at(size_t i) const
{
    assert(!is_diskless());
    return disks_.at(i);
}


void Tower::top_to_top(Tower& dest_tower)
{
    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(size_t disk = 0; disk < num_disks(); disk++) {
        if(T.disk_at(disk).size() != disk_at(disk).size()) return false;
    }
    return true;
}


// Returns the number of disks on the tower in the vector with the most disks
size_t highestTower(const std::vector<Tower>& towers)
{
    assert(!towers.empty());
    size_t 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

#pragma once

#include <cstddef>
#include <vector>

class Tower;

class TowerDrawer {
public:
    TowerDrawer(int pole_height);
    int pole_height() const;
    size_t draw(const Tower&) const;
    size_t draw(const std::vector<Tower>&) const;

private:
    void draw_spaces(const int num_spaces = 1) const;
    int num_slashes(const int disk_size) const;
    int num_chars(const int disk_size) const;
    int center_of(const int disk_size) const;
    void draw_disk_row(const int disk_index, const Tower&) const;
    void draw_rod_row(const Tower&) const;
    void draw_rod_top(const Tower&) const;
    void draw_tower_row(int row, const Tower&) const;


    int pole_height_;
};

TowerDrawer.cpp

#include "TowerDrawer.h"
#include "Tower.h"
#include <cassert>
#include <iostream>
#include <climits>

using namespace std;



TowerDrawer::TowerDrawer(int pole_height): pole_height_(pole_height)
{}


int 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(int i = pole_height_; i >= 0; i--) {
        for(size_t t = 0; t < towers.size(); t++) {
            draw_tower_row(i, towers.at(t));
            draw_spaces(12);
        }
        cout << endl;
    }
    return towers.size();
}


int to_signed(const unsigned x)
{
    if(x <= INT_MAX)
        return static_cast<int>(x);

    if(x >= INT_MIN)
        return static_cast<int>(x - INT_MIN) + INT_MIN;

    throw x; // Or whatever else you like
}


void TowerDrawer::draw_spaces(const int N) const
{
    assert(N >= 0);
    for(int i = 0; i < N; i++) cout << ' ';
}


int TowerDrawer::num_slashes(const int disk_size) const
{
    return disk_size * 2 + 1;
}


int TowerDrawer::num_chars(const int disk_size) const
{
    return 2 + num_slashes(disk_size);
}


int TowerDrawer::center_of(const int disk_size) const
{
    return (num_chars(disk_size) - 1) / 2;
}


void TowerDrawer::draw_disk_row(const int 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_slash_bracket_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();
        cout << "|_|";
        draw_spaces();
        return;
    }
    draw_spaces(center_of(t.size_of_largest_disk()) - 1);
    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);
        cout << '_';
        draw_spaces(2);
        return;
    }
    draw_spaces(center_of(t.size_of_largest_disk()));
    cout << '_';
    draw_spaces(center_of(t.size_of_largest_disk()));
}


// todo: draws funnily when tower.num_disks() == tower.height()
void TowerDrawer::draw_tower_row(int 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: towers

debug: main.cpp parse.cpp move_parser.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp
    g++ -std=c++17 main.cpp parse.cpp move_parser.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp -o towers -g

towers: main.cpp parse.cpp move_parser.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp
    g++ -std=c++17 main.cpp parse.cpp move_parser.cpp help.cpp Disk.cpp Tower.cpp TowerDrawer.cpp -o towers

clean:
    rm towers
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Could you please upload the code somewhere, as a whole project, e.g. on github? \$\endgroup\$ Commented Jan 30, 2023 at 11:23
  • 1
    \$\begingroup\$ Once you fix any issues you are going to fix, you might want to re-post this code as a follow up question. \$\endgroup\$
    – pacmaninbw
    Commented Jan 30, 2023 at 14:57
  • 1
    \$\begingroup\$ This is a small point but I think a useful one. It's helpful to draw the disks so that "odd" and "even" numbered disks can be distinguished. Tower of Hanoi is often used as a motivating example to teach recursive solutions but that has never sat well with me because there is a much simpler nonrecursive solution provided that you can quickly distinguish between odd and even disks. \$\endgroup\$ Commented Jan 30, 2023 at 18:04
  • 1
    \$\begingroup\$ The nonrecursive solution, FYI, is that the first move is obviously to move the topmost disk to an empty pile. After that just follow the algorithm (1) never move the same disk twice in a row, and (2) never move an odd disk onto an even, or even onto odd. Do that until you're done. No tracking recursive state necessary. \$\endgroup\$ Commented Jan 30, 2023 at 18:06
  • 1
    \$\begingroup\$ @EricDuminil, I'll make an effort to do that sometime this week. \$\endgroup\$
    – Mode77
    Commented Jan 30, 2023 at 22:12

2 Answers 2

6
\$\begingroup\$

General Observations

Interesting! Decent job of coding. The partitioning of the code seems pretty good, but it can be improved.

In the display of towers, the towers should be numbered so that the user knows what to input. What might also be helpful to the user is a prompt the indicates the proper format of a move.

What's your next move? [Two integers between 1 and 3]

There should be a -h command line argument that would print the proper usage. Since in the future there may be more arguments on the command line, there should be a command line parser as well. The game could also prompt for the number of disks at runtime if there are no command line arguments.

Be consistent in all of the programming, more on this below.

wc -Llwc *.h *.cpp
15 28 238 42 Disk.h
25 52 577 54 Tower.h
27 72 708 65 TowerDrawer.h
25 71 533 84 help.h
48 163 1267 95 move_parser.h
27 90 710 78 parse.h
47 198 1036 64 Disk.cpp
105 235 2177 77 Tower.cpp
121 276 2928 96 TowerDrawer.cpp
49 276 1868 109 help.cpp
248 648 6691 117 main.cpp
57 180 1613 94 move_parser.cpp
69 225 1869 74 parse.cpp
863 2514 22215 117 total

Allow the compiler to check the code for you, there are some warning messages on the code if you add the proper compiler switches:

-Wall -Wextra -pedantic -Werror

Line 23 in main.cpp: warning: 'return': conversion from 'double' to 'unsigned int', possible loss of data
Line 19 in Tower.cpp warning: 'initializing': conversion from 'size_t' to 'int', possible loss of data

Line 23 in main.cpp :

    return pow(2, num_disks) - 1;

The call to pow() returns a double.

Line 19 in Tower.cpp:

    for (int i = num_disks; i > 0; i--) {

Since num_disks is size_t it might be better of i is size_t as well.

Rather than creating a Makefile you might want to use CMake instead. The current Makefile is ignoring dependencies that it should not be ignoring (the header files). CMake will handle these dependencies for you.

Be Consistent!

There are multiple places where the code is inconsistent, one is the use of include guards in some header files and #pragma once in other header files. Both are legitimate, include guards are older technology and a little more portable. The important thing is to pick one and use it through out the project. Other people might have to maintain the code and consistency is very important in that case.

Use int or unsigned int consistently. There is no clear need for integers on any of the code since most of the values can never go negative.

The function getScore() should probably return either an unsigned int or size_t.

Many/most of the files use the names spaces, but Tower.cpp has using namespace std;.

Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

Partitioning of Functionality

While the code does a fair job of partitioning functionality, the main.cpp still contains code for parsing the commands, all of the parsing related functions should be in parse.cpp with prototypes in parse.h. The point of partitioning the code should be to remove as many dependencies as possible, only global entry points should be listed as prototypes in header files. All of the parsing related enums should be declared in parse.h if they need to be global or parse.cpp if the can be made local.

The function resetTowers() probably belongs in either Tower.h or TowerDrawer.h.

integers Versus size_t

Check the value of argv[1] if it is on the command line, it should never be negative, report a user error if it is and exit the program. That way the code can use just unsigned int or size_t rather than using integers and needing conversions from unsigned to int. Switching back and forth between unsigned and signed can be a cause of bugs.

Line 18 in Tower.cpp is meaningless, since size_t is unsigned num_disks can never be less than zero.

    assert(num_disks >= 0);

Complexity

Some measures of complexity are the number of lines in a function or a section of code and the number of levels of indentation.
One of the best practices in coding is that a function should be completely viewable in a single scree (on my computer that is 58 lines of code or less). The main() function in main.cpp is 112 lines of code, this is almost 2 full screens on most computers. The reason that one screen max per function is a best practice is that it is very difficult for someone reading, writing or maintaining the code to keep track of everything the function is doing.

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

The code in the while (!(requestQuit || gameOver)) loop belongs in its own function, in fact it might be broken up into multiple functions. If I was writing the code I might have a Game class that had a play() function. The instantiation of the Game object would handle most of the setup (hint pass the number of disks into the game constructor). The includes for parse.h, move_parser.h, Tower.h and TowerDrawer.h would move from main.cpp to Game.cpp.

\$\endgroup\$
2
  • \$\begingroup\$ With respect to return pow(2, num_disks) - 1; returning double, the correct fix is not to change the return value of the function to double, since the return value will be an integer, but rather to replace the call to pow with (1U << num_disks) - 1. Avoids the need to cast the result just to suppress a warning, and gives a decent speed boost as a bonus. \$\endgroup\$
    – Ray
    Commented Jan 30, 2023 at 20:45
  • \$\begingroup\$ @Ray Yes, that is one solution. Based on the code in the question I'm not sure the original poster understands shifting and other binary operations. \$\endgroup\$
    – pacmaninbw
    Commented Jan 30, 2023 at 22:05
5
\$\begingroup\$

In addition to pacmaninbw's excellent answer, I'd like to add:

Avoid using system()

system() is very expensive: it has to fork the process, start a shell, interpret the command you gave it, which in turn causes another process to be spawned which does the actual clearing (/usr/bin/clear). It also is non-portable: while it works on most UNIX-like operating systems, it will work on Windows. Also, it is rather unsafe; the user can have modified its environment such that the shell command clear does something else than calling /usr/bin/clear.

Also consider that /usr/bin/clear itself has to be implemented in some language. This happens to be C. So you should be able to clear the screen using C++ code without resorting to system(). On most operating systems (including Windows 10 and later) you can do this by sending an ANSI escape code to the standard output:

std::cout << "\033[2J";

About completely trivial functions

While it's often good to encapsulate even very simple code into a function, some of your functions are so trivial that it might be better not to have them at all. Things like printStatus() for example, where just writing:

std::cout << "\n" << statusMessage << "\n\n";

Is virtually no more work than writing:

printStatus(statusMessage);

You can immediately see that statusMessage is passed to std::cout, so you can see that the first line prints the message.

A function like this would have merit if it would allow you to change the way you print the status. For example, you want to make a GUI instead of a console program, and you want to update a statusbar widget instead of printing to std::cout. However, for that to work your code should always call functions to update what is on the screen, but now you have some functions that do this, and other times you just std::cout << "something" directly. As pacmaninbw already mentioned: be consistent!

The same goes for askQuestion(). This function would have made sense if it would actually read a line from the input after printing the question, and return it.

Avoid C functions if there are better C++ functions

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.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.