when is it ok to use goto?

[MarketAnarchist]

While working on my chess program i wrote a printBoard() function where i really cant imagine any more efficient way to write this function than using a goto.

I have been told that its never ok to use goto's. This paper http://www.u.arizona.edu/~rubinson/copyright_violations/Go_To_Considered_Harmful.html has gone so far as to claim that goto should be removed from higher level programing languages all togather.

so if this paper is to be believed than surely there must be a more efficient way to write this function. if so than im really curious, what is it?

#include <iostream>

class CBPawn
{
public:
	int iCoord[2];
} qBPawn[8] = {{1,0},{1,1},{1,2},{1,3},{1,4},{1,5},{1,6},{1,7}}; //creates pawns

void printBoard() {
	for (int i1=0; i1<8; ++i1) {
		std::cout << std::endl << " ";
		for (int i2=0; i2<8; ++i2) {
			for (int i3=0; i3<8; ++i3) {
				if (qBPawn[i3].iCoord[0]==i1 && qBPawn[i3].iCoord[1]==i2) {
					std::cout << " BP"; //prints pawns
					goto summonRaptor;
				}
			}
			std::cout << " OO"; //prints blank spaces
			summonRaptor:;
		}
		std::cout << std::endl;
	}
	std::cout << std::endl << "Input: ";
}

int main() {
	printBoard();
    system("pause");
}

here is the best thing i can come up with without using goto, i think the goto code is still better. 28 lines vs 34 lines

#include <iostream>

class CBPawn
{
public:
	int iCoord[2];
} qBPawn[8] = {{1,0},{1,1},{1,2},{1,3},{1,4},{1,5},{1,6},{1,7}};

void printBoard() {
	for (int i1=0; i1<8; ++i1) {
		std::cout << std::endl << " ";
		for (int i2=0; i2<8; ++i2) {
			bool b=false;
			for (int i3=0; i3<8; ++i3) {
				if (qBPawn[i3].iCoord[0]==i1 && qBPawn[i3].iCoord[1]==i2) {
					b=true;
				}
			}
			if (b) {
				std::cout << " BP";
			} else {
				std::cout << " OO";
			}
		}
		std::cout << std::endl;
	}
	std::cout << std::endl << "Input: ";
}

int main() {
	printBoard();
	system("pause");
}

[Codeez]

Just because it shortens code doesn't make it better.

Consider jumping forward; it is an error to jump over an initialization:

goto jump_label;

int x = 256; // error

jump_label:

++x; // error - we jumped its declaration 

Goto's are harder to read as humans <enter spaghetti reference>, so you should always use an alt. method.

Btw your goto label does not need a semi-colon, just summonRaptor: will do. Try naming your for loop variables differently too, I thought I was seeing 11, 12 & 13! :)

[MarketAnarchist]

i get an error when i take the semicolon out.

[Codeez]

You're right. IDK why though. My guess though is a goto expects a statement after the goto label in the same block. That semicolon is nothing to do with the goto, it's just serving as a null statement. All this just rings more alarm bells to not use goto.

[MatthewRock]

You can replace goto with loops and simple break, + one if statement, if I'm not wrong.

Anyway, gotos aren't easily readable and can be hard to debug, so yeah, it's better not to use them.

[JLBorges]

When you find yourself using a goto, the sane approach is to reexamine the structure of your code; try to refactor it to make the code easier to read, understand and maintain.

Modifying the same code to use a flow-control construct like a while loop (a 'boolean controlled while loop' is what people who abhor goto usually advocate), with assorted break, continue or deeply nested if statements thrown in to somehow avoid that goto, does not improve the code structure. As you have experienced in your case, in some cases it makes the code worse - makes it less easy to read, understand and maintain.

For instance, refactoring to use a data-driven approach (usually a good option when we find that the code structure is convoluted) might lead us to something like:

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

struct pawn
{
    int x, y ;
    static constexpr const char* const pic = "BP" ;
};

pawn black_pawns[] = {{1,0},{1,1},{1,2},{1,3},{1,4},{1,5},{1,6},{1,7}}; //creates pawns

constexpr int N = 8 ;

std::ostream& print_board( std::ostream& stm = std::cout )
{
    static const char* const empty = "OO" ;
    static const std::vector<std::string> empty_row( N, empty ) ;

    std::vector< std::vector<std::string> > board( N, empty_row ) ;

    for( pawn p : black_pawns ) board[ p.x ][ p.y ] = pawn::pic ;

    for( const auto& row : board )
    {
        for( const auto& s : row ) stm << ' ' << s ;
        std::cout << "\n\n" ;
    }

    return stm ;
}

int main()
{
	print_board() ;
}

[vlad from moscow]

It is well known that the more lower qualfication pf a programmer the more useful he finds statement goto.:)
You should simply forget that there is such a statement in C++.

[z05DSL3A]

MarketAnarchist wrote:

when is it ok to use goto?

Don't use goto until you have learnt enough to know why using goto is 'bad', then you will have learnt enough to us goto properly.

[Group of Eight]

On a side note, I would like to add that it is good programming practice to make the member functions of a class private.

[vlad from moscow]

@Grey Wolf

Don't use goto until you have learnt enough to know why using goto is 'bad'

If you have learnt enough you will never use goto.:)

[andywestken]

Your function looks like it's probably going in the wrong direction, to me. Esp. if it's supposed to handle pieces in addition to pawns, like most games of chess. Or is your program for chess puzzles which only use black pawns??

But following through with your approach...

The switch from the goto to the "bool b" approach is an ok move, but a slight variation of this might be better. Here a bool value is used to keep track of whether or not the square has been filled, which means the code can be extended to handle other types.

void printBoard() {
    for (int i1=0; i1<8; ++i1) {
        std::cout << std::endl << " ";
        for (int i2=0; i2<8; ++i2) {
            bool isBlank=true;
            for (int i3=0; i3<8; ++i3) {
                if (qBPawn[i3].iCoord[0]==i1 && qBPawn[i3].iCoord[1]==i2) {
                    std::cout << " BP";
                    isBlank=false;
                    break;
                }
            }
            if(isBlank) {
                for (int i3=0; i3<8; ++i3) {
                    if (qWPawn[i3].iCoord[0]==i1 && qWPawn[i3].iCoord[1]==i2) {
                        std::cout << " WP";
                        isBlank=false;
                        break;
                    }
                }
            }
            /*
            Repeat for black and white knights, bishops, rooks, queens, and kings
             */
            if (isBlank) { // if no piece was found in square
                std::cout << " OO";
            }
        }
        std::cout << std::endl;
    }
    std::cout << std::endl << "Input: ";
}

With the repeated i3 loops it's now clearer that the outer loops (i1, i2) are doing one thing (iterating over the squares of the board) whereas the inner loop (i3) is working out if a piece is of a particular type, i.e.

for each square of the board
if the piece is a black pawn
  show "BP"
else if the piece is a white pawn
  show "WP"
<<etc>>
else
  show "00"

So you could (and prob should) factor the inner loops out into a functions, e.g. isBlackPawn.

bool isBlackPawn(int i1, int i2) {
    for (int i=0; i<8; ++i) {
        if (qBPawn[i].iCoord[0]==i1 && qBPawn[i].iCoord[1]==i2) {
            return true;
        }
    }
    return false;
}

bool isWhitePawn(int i1, int i2) {
    for (int i=0; i<8; ++i) {
        if (qWPawn[i].iCoord[0]==i1 && qWPawn[i].iCoord[1]==i2) {
            return true;
        }
    }
    return false;
}

// etc

void printBoard() {
    for (int i1=0; i1<8; ++i1) {
        std::cout << std::endl << " ";
        for (int i2=0; i2<8; ++i2) {
            if (isBlackPawn(i1,i2)) {
                std::cout << " BP";
            if (isWhitePawn(i1,i2)) {
                std::cout << " WP";
            /* etc */
            } else {
                std::cout << " OO";
            }
        }
        std::cout << std::endl;
    }
    std::cout << std::endl << "Input: ";
}

But this approach is getting quite long winded, even when functions are used to tidy up printBoard. So I think you need to take a step back and rethink how you handle your pieces. And the board!

Andy

[MikeyBoy]

On a side note, I would like to add that it is good programming practice to make the member functions of a class private.

Um... what? If you make all the member functions of a class private, then it has no interface! How can the parts of your code that need to use the class do so, if all the methods are private?

[z05DSL3A]

I think he means member data, I hope he means member data.

[Lowest0ne]

When I did chess I had two arrays, the pieces and the board. Having the piece array allowed for a statement such as:
pieces[ BLACK_KING ];, while the board offered board.at( 0, 0);