Empty accessors and mutators are a code smell.

If you need to do some logic that ensures that the class's invariants are upheld when changing a variable, go ahead and wrap it in this way. Otherwise, if modifying the variable arbitrarily can't break the class from the outside, just use public member data.

Getting a bit away from your specific question, I'd caution that part of the problem is that you're beginning to have a rather monolithic, unfocused class.

It's difficult to say without knowing more about the design of the game itself, but the impression that the accessor/mutator names give me is that some of them could stand to be combined into their own class (For example: posX, PosY, and [maybe] Direction might become a class) and others either seem to be misplaced (InFront, InFort, InShop, InSpace) and/or might indicate some kind related state that could be expressed as an enumeration (if states are mutually exclusive or have limited but well-known combinations) or a bitfield (if states can be combined more freely).

Still others might be better solved in a much more general fashion, for example you have a number of variables that seem to track inventory -- if the variety of available inventory items is small this approach may be fine, but if the varieties grow to more than a couple of handfuls, you might want to look into creating an inventory object that holds the actual items, and having this "player" class have an inventory (either directly, or by owning a unique key within a shared inventory system). In short, as you add more variety of items, you tend to want something that starts to look more and more like a database, rather than an ad-hoc collection of values. You can do stats similarly.

Basically, try to think about the different contexts in which you currently plan to access your "player" class, and see how the data naturally groups together -- for example, its unlikely that a single context would care about the player position *and* the contents of their inventory simultaneously, in a well-designed system.

How do you mean 'used solely for other classes'? Like you have class A and class B and class A is the only class that uses class B? If that's what you mean then one option is to nest class B within class A. Here's some more information.

What do you mean? Wouldn't that mean just any variable that's actually changed? How do you define arbitrary when modifying something? Or breaking the class from the outside, for that matter?

For example, suppose you have a class that represents a book that you can write in. That class has a variable "NumWrittenPages" which tells you how many pages have been written in. It also has a function "WriteOnPage" which increments the NumWrittenPages variable.

Now; suppose some other code just goes and sets NumWrittenPages to some new number, without calling WriteOnPage. This would "break" the class in the sense that the object no longer correctly reflects reality. If WriteOnPage depends on the value of NumWrittenPages, this is probably a bug waiting to happen.

In that case, you want to use an accessor for NumWrittenPages, but not provide a mutator.


Or you might have a NormalizedVector class, which expects that its length is always 1. Modifying its x, y, and z coordinates directly is a recipe for disaster, because external code can make the length change to anything. So you wouldn't make them public. However, you might want to support setting the vector up all at once, such as Set(x, y, z). In that case, you simply implement Set() so that it re-normalizes the vector regardless of what the input numbers are. This ensures that the class invariant (i.e. the vector is length 1) is never violated.


In cases like this where you need to ensure something doesn't break when you set new variable values, mutators are OK. If you don't need limitations like that, just use public member variables.

Here's an idea for you: "void AddToCharacteristic( CharacteristicType type, int value );"