Example 2 is quite bad for testing... and I don't mean that you can't test the internals. You also can't replace your XmlReader object by a mock object as you have no object at all.
Example 1 is needlessly hard to use. What about
XmlReader reader = new XmlReader(url);
Document result = reader.getDocument();
which is not any harder to use than your static method.
Things like opening the URL, reading XML, converting bytes to strings, parsing, closing sockets, and whatever, are uninteresting. Creating an object and using it is important.
So IMHO the proper OO Design is to make just the two things public (unless you really need the intermediate steps for some reason). Static is evil.
I think you may be getting a little bogged down in the English meaning of state, when compared to the State Pattern (or Finite-State Machine, which is really a diagrammatic representation of a State Pattern). Both are appropriate here, but they shouldn't be confused.
The State Pattern is something which should, given various common stimuli, operate on the game state differently at different times in the game. So, as you've rightly concluded, the lobby stage (which I'm assuming to be while people are joining a game) is a State in that context.
The game state is a list of players, pieces, cards, dice, whatever else, and all the information required to simulate them. You might want to call that the context.
Each State should receive a UserClicked message from the application which receives the context and coordinates or area clicked and should operate on those accordingly. The information that you are trying to give ownership of to the State object does not belong there, it belongs in the game itself, alongside the State.
The state may also have a UserPressedKey method, a TimerTicked method or any other kind of stimulus on which it should act. But each of these should act on the game state rather than being the game state.
One important method of the State will be ScreenRefresh, which will draw the context for the user.
Here is a very rough example:
abstract class ApplicationState {
void Begin(GameContext context);
void UserClicked(GameContext context, int x, int y);
void UserPressedKey(GameContext context, char key);
void TimerTicked(GameContext context);
void ScreenRefresh(GameContext context);
protected void OnStateChanged(ApplicationState newState) {
// inform interested parties that state has changed,
// using the observer pattern
}
}
class LobbyState : ApplicationState
{
void Begin(GameContext context) {
context.Players = new User[context.NoOfPlayers];
}
void UserClicked(GameContext context, int x, int y) {
// Find a displayed player box which contains click coords
int index = -1;
for (i=0; i < context.Players.Length; i++) {
if (PlayerBox[i].Contains(x, y)) {
index = i;
}
}
if (index > -1) {
if (context.Player[index] == null) {
context.Player[index] = context.ActivePlayer;
} else if (context.Player[index] == context.ActivePlayer()) {
context.Player[index] = null;
} else {
ErrorSound.Play();
}
} else if (CloseIcon.Contains(x, y)) {
OnStateChanged(new ExittingState());
} else {
ErrorSound.Play();
}
}
void UserPressedKey(GameContext context, char key) {
if (key = 'J' or key = 'j') {
if (! context.AddPlayerRandomly(context.ActiveUser)) {
ErrorSound.Play();
}
if (context.GameIsNowFull()) {
OnStateChanged(new InitializingState());
}
} else if key = Esc {
if (!context.RemovePlayer(context.ActiveUser)) {
ErrorSound.Play();
}
} else {
ErrorSound.Play();
}
}
void TimerTicked(GameContext context) {
// there is no use for a game timer while
// we're trying to fill the game, but you
// want one to tick anyway, because the
// application doesn't know which state
// it is in.
}
void ScreenRefresh(GameContext context) {
DrawPlayerBoxes(context.Screen, context.Players);
DrawExitIcon(context.Screen);
}
}
class InitializingState : ApplicationState {
private double waitSpinnerAngle = 0;
void Begin(GameContext context) {
context.RandomizePlayOrder();
context.ShuffleCards();
// all other game initialization rules here
OnContextChanged(new PlayerUpState(context.Players[0]));
}
void UserClicked(GameContext context, int x, int y) {
// Not responding to user input for a moment
ErrorSound.Play();
}
void UserPressedKey(GameContext context, char key) {
// Not responding to user input for a moment
ErrorSound.Play();
}
void TimerTicked(GameContext context) {
waitSpinnerAngle += 0.05;
if (waitSpinnerAngle > 1) waitSpinnerAngle = 0;
}
void ScreenRefresh(GameContext context) {
DrawWaitSpinner(waitSpinnerAngle);
}
}
See where I'm going with this? The Application itself is then as simple as setting the initial State, hooking a listener into the OnStateChanged event and calling Begin. When the listener hears that the event is called, unhook your old State, hook in the new one, and call Begin again.
Everything else is triggered by events from the mouse, keyboard or timer, passed directly to the current State without knowledge of which State the game is currently in. If you are running this game across a network then you will also need an event for changes of state received from other players.
Everything in your State and Context is now very unit-testable and separation of concerns are observed. Although, you may want to refactor that LobbyState#UserClicked method a bit, among other things.
Remember, this is just a free example and you get what you paid for it. Don't try to apply it directly to your game. Just use it to understand how the State Pattern should work in the context of a game.
Best Answer
You decorate a method with
[Pure]:If the method doesn't have side effects. For example, if the method accesses a database and modifies it or its result depends on the database, it's not pure.
And if you expect to use it in code contracts. For example, if the method is pure, but you have no intention to use it in code contracts, adding
[Pure]would have no benefit and won't make your code faster.As far as it concerns constructors, it looks like they are assumed to be pure in .NET and don't need an explicit attribute. I looked at several constructors in .NET Framework source, such as
DateTime, and they don't have[Pure]attribute.I suppose this is done for several reasons:
It may be too impractical to have to write a parameterless constructor with
[Pure]attribute just to be able to use the class/struct in a contract.Some, such as
String, don't have explicit constructors.The constructors receive special treatment even outside code contracts; for example, you're not expected to throw exceptions inside them.
[Pure]is just a convention which is here to simplify your life, but there is no actual static checking to ensure that the method decorated with this attribute is pure.void DestroyDatabase()may be decorated as being pure, and code contracts won't notice anything wrong..NET Framework itself contains constructors which are not pure. For example,
List<T>(IEnumerable<T> collection)is actually impure if looping through the collection has side effects.Contracts shout be kept simple. I can easily imagine a contract such as
Contract.Requires(!string.IsNullOrEmpty(name)), so there are good reasons to declare the staticstring.IsNullOrEmptypure.On the other hand, if you need a
StringBuilderin order to build the string you'll check then for something by calling an instance method of your business class, you're probably misusing the contracts. That's also whyStringBuilder.ToStringis not marked as pure, even if it might be (is it?)