Quote:Original post by Steve Elliott
Thanks guys. :-D
Can't help but think there's a better way of designing my classes - but that's ok for now.
Of course there is. :)
Updating the board
is not logically a part of the process of getting input,
so don't put that code there. The logical way to do things is to
return the input from a function that "gets the input", and pass it along (as an argument) to the board. You can handle "is the game still going on?" by
asking, not telling - there's no particularly good reason to communicate that state through a global. Now, between "input" and "board", which "knows" whether it's possible to keep playing? I would argue that the board would know, in general. So it makes sense to return that information from the update function.
Actually, it looks like the reason you want the Input class to "talk to" the board is to read its raw data, in order to figure out what piece is being moved - and then to tell the board that same information when you call update(). That's missing the point. Don't ask objects what they aren't specifically designed to tell you, and don't tell them things they know.
In fact, you probably don't need an input class at all. The board should be responsible for figuring out whether a particular move is legal, because it's the board which has the necessary information. Just get the input, and send it along. Design the input spec such that you don't need to know anything about the game logic - the board will check, and report whether the move was actually made, or was illegal.
It seems like you have some other fundamental misunderstandings, though - in particular, your classes look like they contain redundant data. The usual rules about scope apply: use a local when you can, and a class member when you have to (i.e. when it's actually logically "part of the object").
Consider dropping the Hungarian notation, too. It's actually seducing you into choosing worse "non-Hungarian-notation parts" of the variable names here. The positions on a Board aren't Boards; if you were to just write Board instead of csBoard and iBoard (not to mention pBoard and obBoard), this would stand out as kinda wrong -
which it is.
// Splitting into separate files left as an exercise, of course. ;)// I am using old-style comments to represent the bits you need to fill in,// and regular ones to explain what is going on here.enum { ILLEGAL_MOVE, GAME_STILL_GOING, YOU_WIN, YOU_LOSE, DRAW } GameStatus;class Board { // Don't make the data public! That's why we have this kind of // organization in the first place; so we can *control* how 'cells' // are modified. int cells[64]; public: // I assume the pointer pointed at the cells; that's totally redundant // anyway - C++ lets you use the name of an array as if it were a // pointer to the beginning element. Board(); // From what is shown, it is extremely unlikely that you need a // destructor at all. GameStatus update(int from, int to); void draw();};GameStatus Board::update(int from, int to) { // Of course, "the complexity has to go somewhere": // we need to put the code here that determines whether the move // is legal, and what piece is being moved. But since 'cells' is // OUR data, we are perfectly positioned to do so. if (/* move is not legal - maybe split into another function */) { return ILLEGAL_MOVE; } /* make the move */ /* return one of the other values accordingly */}void Board::draw() { /* As before. */}#include <iostream>using namespace std;int main() { Board board; bool playing = true; do { board.draw(); int from = /* ask for input here */; int to = /* ask for input here */; switch (board.update(from, to)) { case ILLEGAL_MOVE: /* complain */; break; case GAME_STILL_GOING: break; /* nothing to do */ case YOU_WIN: /* report victory */; playing = false; break; case YOU_LOSE: /* report defeat */; playing = false; break; case DRAW: /* report stalemate */; playing = false; break; } } while (playing);}
Yes, It's Really That Easy(TM).
