2

I'm writing a minesweeper game with curses. To represent each tile, I wrote small Tile class that contains information about the possible state of each tile (whether or not it's flagged, whether or not it has a bomb under it, and whether or not it's "hidden"). This is the interface so far:

class Tile {
    bool hidden;
    bool containsBomb;
    bool hasFlag;

public:
    Tile(bool startWithBomb, bool startHidden = true);

    bool isHidden() const;
    void makeHidden();
    void unhide();

    bool hasBomb() const;
    void placeBomb();
    void removeBomb();

    bool hasFlag() const;
    void placeFlag();
    void removeFlag();

};

I could have made the setters for each a single function, but I think them being separate will lead to more readable code (tile.setFlag(false) vs tile.removeFlag())

My problem is, this can potentially lead to "bad states". An unhidden tile shouldn't be able to contain a flag for example.

I was reading over this post, and several users "bash" using getters/setters; with good reasons.

My question is: Before I get farther in designing this class, is there a better way to set this up to avoid using generic getter/setters for each private flag?

3
  • I read over the linked post and the anti getter/setter article attached. Seems like C++ and C# have very different philosophies.
    – Stephen
    May 8, 2015 at 4:59
  • @Stephen Yes. AFAIK, c# has shortcuts for writing getters/setters. May 8, 2015 at 13:15
  • even without those shortcuts it's always been considered good practice to expose properties, even with simple getters/setters.
    – Stephen
    May 11, 2015 at 0:09

2 Answers 2

3

Yes, having exactly one getter/setter pair per private flag is a code smell. If you aren't providing a simple interface to hide some implementation details, you're not benefiting very much from using a class.

My main concrete suggestion would be to represent the internal state in a way that makes "bad states" theoretically impossible. For instance:

class Tile {
    enum DisplayState { DEFAULT, FLAGGED, KNOWN_EMPTY };
    DisplayState displayState;
    bool containsBomb;
    ...
};

Notice that containsBomb can be true or false in any of the three states, so it makes sense to keep that member separate. It's up to you whether things like calling removeFlag() on a KNOWN_EMPTY tile should throw an exception or simply do nothing.

Now the big-picture issue. If the part of the code that uses Tile objects really does need all of these methods, i.e. full access to and control of the Tile's entire state, then it's not really clear how you benefit from making Tile a class (as opposed to a POD struct) in the first place. Perhaps it would be better to have a Board class with a few 2D arrays of booleans to represent all the flags/bombs/etc, since that class would be able to hide a lot of iteration logic from its users. I can't say for sure if that'd be an improvement without reading all of your code, but hopefully that helps.

6
  • I was going to make it a 2D array of Tiles. If the user clicks on a tile, the Board would issue a command to the clicked Tile. What is a POD Struct? Thank you for your response. May 7, 2015 at 20:06
  • POD = Plain Old Data. The technical definition is complicated (nothing is simple in C++), but in this case I'm using it to mean a struct with just members and no methods.
    – Ixrec
    May 7, 2015 at 20:09
  • Oh, like using the class as a "bag of data"? May 7, 2015 at 20:09
  • Exactly. Regarding the first half of your comment: Is it simpler for the Board to implement setFlag() as flagsArray[x][y] = true, or as tilesArray[x][y].setFlag(true)? Again, I can't know for sure, but my guess would be that the former has a slight edge.
    – Ixrec
    May 7, 2015 at 20:11
  • I just kind of dived in with general ideas on how I wanted it to work, so I'm not sure exactly which would be cleaner yet. I'm probably going to need to do some refactoring down the road. Wouldn't storing all the data in one array be more efficient than storing bools across a few array representing different parts of the state of the tile? I know vector has the specialization for bools, but that would still require each update to iterate over 3 arrays instead of 1. May 7, 2015 at 20:15
2

My problem is, this can potentially lead to "bad states".

The opposite is true: The setter allows you to prevent "bad states". You are the one to define the behaviour of the state machine, by specifying what happens when one variable changes.

An unhidden tile shouldn't be able to contain a flag for example.

Therefore, calling unhide() should set hasFlag to false and placeFlag() should only set hasFlag to true if hidden is true.

There is no such thing as a "bad state" because you should define your state machine well and handle every case. These setters are just the implementation of that state machine.

Reaching a "bad state" is a flaw in the transfer function.

1
  • 1
    Right. The reason for making private variables is to control how they are used with an interface you define. Make a good interface, don't expose the hidden members!
    – Alan Wolfe
    May 9, 2015 at 4:37

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.