7
\$\begingroup\$

Inspired by Mayur Kulkarni's question this morning, I decided to post some code for the classic Towers of Hanoi problem, but this time in C++. Since this is fairly "textbook" kind of task, all suggestions are welcome.

#include <iostream>
#include <vector>
#include <string>
#include <iterator>

class Hanoi {
    typedef std::vector<int> Tower;
    Tower a, b, c;

    void show(std::string const &label, Tower c) {
        std::cout << label;
        std::copy(c.cbegin(), c.cend(), std::ostream_iterator<int>(std::cout, "\t"));
        std::cout << "\n";
    }

    void dump() {
        show("A: ", a);
        show("B: ", b);
        show("C: ", c);
        std::cout << "\n";
    }

    void MoveDisc(Tower &a, Tower &b) {
        b.push_back(a.back());
        a.pop_back();
    }

    void MoveStack(Tower &a, Tower &b, Tower &c, size_t size) {
        if (size == 0)
            return;
        MoveStack(a, c, b, size - 1);
        MoveDisc(a, b);
        dump();
        MoveStack(c, b, a, size - 1);
    }
public:
    Hanoi(std::initializer_list<int> input) : a(input) { dump(); }

    void operator()() { MoveStack(a, c, b, a.size()); }
};

int main() {
    Hanoi h { 30, 25, 20, 15, 10, 5 };
    h();    
}
\$\endgroup\$
4
\$\begingroup\$

Here are just a few observations that may help you improve your code. In all, it is quite clean and easy to read and understand.

Use const where practical

The show() and dump() function don't (and shouldn't) alter the underlying object, so they should be declared const:

void show(std::string const &label, Tower c) const;
void dump() const;

Use reserve() to prevent reallocations

The constructor can easily determine the maximum size of the vectors. If there were much larger ones than this, one could avoid potentially costly memory reallocations by using Tower.reserve() within the constructor.

Prefer flexible output

This is admittedly a very simple program, but generally it's nice to allow for printing to any std::ostream rather than having std::cout hardcoded into the class. Perhaps it could be a constructor argument.

Use all required #includes

The code makes use of std::initializer_list but doesn't have this line:

#include <initializer_list>

However, as @Morwenn points out in a comment, it's not needed to include it explicitly since std::vector must necessarily already include it.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'm somewhat curious: does the fact that std::vector has a ctor that accepts an initializer_list mean that including <vector> necessarily declares initializer_list? \$\endgroup\$ – Jerry Coffin Jan 10 '16 at 1:36
  • \$\begingroup\$ It's a good question. Probably so, but I'd be inclined to include it anyway because its explicitly used. \$\endgroup\$ – Edward Jan 10 '16 at 2:54
  • 1
    \$\begingroup\$ @JerryCoffin The synopsis of every container header in the latest draft has an explicit #include <initializer_list>, even <array> which doesn't even use it IIRC. So it's guaranteed that including <vector> also includes <initializer_list>. \$\endgroup\$ – Morwenn Jan 10 '16 at 13:33
  • \$\begingroup\$ @Morwenn: thanks for checking. I've included that into my answer. \$\endgroup\$ – Edward Jan 10 '16 at 14:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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