Tower of Hanoi (without recursion)

I came across an interesting method of solution for the Tower of Hanoi puzzle
and wrote a short version of it as a programming exercise.

The program produces the correct results but I have two questions.

First is there simpler way to write the alternating step of determining the
only valid move which does not involve the smallest disk.

Second when I try to make the two primary routines (move smallest disk and
make alternating move) into functions the handling of variables becomes
unwieldy.

```
/* tower.c

Tower of Hanoi -- mechanical solution

Place one of the three rods upright at each corner of a triangle.

Alternate between moving the smallest disk and making the only valid move
which does not involve the smallest disk.

The smallest disk always moves in the same direction: counter-clockwise if
there are an odd number of disks in the puzzle; clockwise if there are an even
number of disks in the puzzle.

from "The Icosian Game and the Tower of Hanoi" in THE SCIENTIFIC AMERICAN BOOK OF
MATHEMATICAL PUZZLES & DIVERSIONS by Martin Gardner, (Simon and Schuster, 1959),
pp. 55 - 62
*/

#include
#include

int destinationCount (int array[], int numberOfElements)
{
int count = 0, i;

for ( i = 1; i 3 )
rodTo = 1;
if ( rodTo = 1; --i )
topDisk[rod[i]] = i;

// find which disk to move

switch ( rod[1] )
{
case 1:
rodFrom = 2;
rodTo = 3;
break;
case 2:
rodFrom = 1;
rodTo = 3;
break;
case 3:
rodFrom = 1;
rodTo = 2;
break;
default:
printf ("error");
break;
}

if ( topDisk[rodFrom] > topDisk[rodTo] ) {

Remove the everyOtherMove flag variable

You use it only once to control the logic of your program:

if ( ! everyOtherMove ) {

and this usage may be substitued with a evenness check of the moveCount variable that you would have anyway.

Do not declare vars so much before using them

At line 36 temp is declared as an int, at line 113 temp is used for the first time. How can the reader remember the type of temp 77 lines later? Declare it just before using it.

Loop variables should be declared inside the loop statement as C99 allows it.

Use ternary when it clearly simplifies

if ( (numberOfDisks & 1) == 0 )
    smallestDir = 1;
else
    smallestDir = -1;

Becomes

int smallestDir = (numberOfDisks & 1) == 0 ? 1 : -1

Much shorter and without the smallestDir = repetition.

Using % instead of bitwise operations to check evenness would be a further improvement.

Reduce main (both code and vertical whitespace)

I introduce this helper function:

int wrap_around(int min, int max, int value)
{
    return value  max ? min : value);
}

(Please note that it could also be written with if conditionals, I wrote it like this just because of my familiarity with ternary).

The first if branch now is:

if (moveCount % 2 == 0) {
        // move smallest disk
        rodFrom = rod[1];
        rodTo = wrap_around(1, 3, rodFrom + smallestDir);
        disk = 1;
    }

while before it was:

if ( ! everyOtherMove ) {

        // move smallest disk

        rodFrom = rod[1];

        rodTo = rodFrom + smallestDir;
        if ( rodTo > 3 )
            rodTo = 1;
        if ( rodTo < 1 )
            rodTo = 3;

        disk = 1;

    }

The same concept is expressed in much less space, and this is a good attribute in my view because:

• Some logic is modularized in other functions, the reader gets a more abstract overview.

• If all code is compacted this way an overall view of the program becomes possible helping understanding.

Compacting the else clause

else {

        // make only valid move not involving the smallest disk

        // find disk at the top of each rod

        for ( i = 1; i = 1; --i )
            topDisk[rod[i]] = i;            

        // find which disk to move

        switch ( rod[1] )
        {
            case 1:
                rodFrom = 2;
                rodTo = 3;
                break;
            case 2:
                rodFrom = 1;
                rodTo = 3;
                break;
            case 3:
                rodFrom = 1;
                rodTo = 2;
               break;
            default:
                printf ("error");
                break;
        }            

        if ( topDisk[rodFrom] > topDisk[rodTo] ) {
            // swap values
            temp = rodFrom;
            rodFrom = rodTo;
            rodTo = temp;            
        }

        disk = topDisk[rodFrom]; 

    }

I do not fully understand the uppermost two for loops but both the switch and the body of the if ( topDisk[rodFrom] > topDisk[rodTo] ) statement are performing very clear, specific tasks, so:

else {
        // make only valid move not involving the smallest disk
        // find disk at the top of each rod
        for ( i = 1; i = 1; --i )
            topDisk[rod[i]] = i;            

        // find which disk to move
        find_start_and_destination(rod[1], *rodFrom, *rodTo);
        if ( topDisk[rodFrom] > topDisk[rodTo] ) {
            SWAP(rodFrom, rodTo);    
        }
        disk = topDisk[rodFrom]; 

    }

I just removed the unnecessary blanklines (blanklines should separate logically separated blocks of code, not each line / statement), and used a function to incorporate the switch and a macro to swap variables. The function must use pointers because two values may not be returned from a function in C, but I think the modularization is still an advantage.

StackExchange Code Review Q#137997, answer score: 4