9

In this series of blog posts, Eric Lippert describes a problem in object-oriented design using wizards and warriors as examples, where:

abstract class Weapon { }
sealed class Staff : Weapon { }
sealed class Sword : Weapon { }

abstract class Player 
{ 
  public Weapon Weapon { get; set; }
}
sealed class Wizard : Player { }
sealed class Warrior : Player { }

and then adds a couple of rules:

  • A warrior can only use a sword.
  • A wizard can only use a staff.

He then goes on to demonstrate the problems you run into if you try to enforce these rules using the C# type system (e.g. making the Wizard class responsible for making sure that a wizard can only use a staff). You violate the Liskov Substitution Principle, risk run-time exceptions or end up with code that is difficult to extend.

The solution he comes up with is that no validation is done by the Player class. It is only used to track state. Then, instead of giving a player a weapon by:

player.Weapon = new Sword();

state is modified by Commands and according to Rules:

...we make a Command object called Wield that takes two game state objects, a Player and a Weapon. When the user issues a command to the system “this wizard should wield that sword”, then that command is evaluated in the context of a set of Rules, which produces a sequence of Effects. We have one Rule that says that when a player attempts to wield a weapon, the effect is that the existing weapon, if there is one, is dropped and the new weapon becomes the player’s weapon. We have another rule that strengthens the first rule, that says that the first rule’s effects do not apply when a wizard tries to wield a sword.

I like this idea in principle, but have a concern about how it might be used in practice.

Nothing seems to prevent a developer from circumventing the Commands and Rules by simply setting the Weapon on a Player. The Weapon property needs to be accessible by the Wield command, so it can't be made private set.

So, what does prevent a developer from doing this? Do they just have to remember not to?

  • 2
    I don't think this question is language specific (C#) as it is really a question about OOP design. Please consider removing the C# tag. – Maybe_Factor Nov 23 '17 at 0:03
  • 1
    @maybe_factor c# tag is fine because the code posted is c#. – CodingYoshi Nov 23 '17 at 2:08
  • Why don't you ask @EricLippert directly? He seems to appear here on this site from timt to time. – Doc Brown Nov 23 '17 at 6:54
  • @Maybe_Factor - I did waver over the C# tag, but decided to keep it in case there is a language-specific solution. – Ben L Nov 23 '17 at 8:56
  • 1
    @DocBrown - I did post this question on his blog (only a couple of days ago admittedly; I haven't waited that long for a response). Is there a way to bring my question here to his attention? – Ben L Nov 23 '17 at 8:59
9

The whole argument that series of blog posts leads up to is in Part Five:

We have no reason to believe that the C# type system was designed to have sufficient generality to encode the rules of Dungeons & Dragons, so why are we even trying?

We have solved the problem of “where does the code go that expresses rules of the system?” It goes in objects that represent the rules of the system, not in the objects that represent the game state; the concern of the state objects is keeping their state consistent, not in evaluating the rules of the game.

Weapons, characters, monsters and other game objects are not responsible for checking what they can or can not do. The rule system is responsible for that. The Command object is not doing anything with the game objects either. It just represents the attempt to do something with them. The rule system then checks if the command is possible, and when it is the rule system executes the command by calling the appropriate methods on the game objects.

If a developer wants to create a second rule system which does things with characters and weapons which the first rule system would not allow, they can do that because in C# you can not (without nasty reflection hacks) find out where a method-call comes from.

A workaround which might work in some situations is putting the game objects (or their interfaces) in one assembly with the rule engine and mark any mutator-methods as internal. Any systems which need read-only access to game objects would be in a different assembly, which means they would only be able access the public methods. This still leaves the loophole of game objects calling each other's internal methods. But doing that would be obvious code smell, because you agreed that the game object classes are supposed to be dumb state-holders.

4

The obvious problem of the original code is that it is doing Data Modeling instead of Object Modeling. Please note that there is absolutely no mention of actual business requirements in the linked article!

I would start by trying to get actual functional requirements. For example: "Any player can attack any other players, ...". Here:

interface Player {
    void Attack(Player enemy);
}

"Players can wield a weapon which is used in the attack, Wizards can wield a Staff, Warriors a Sword":

public class Wizard: Player {
    ...
    public void Wield(Staff weapon) { ... }
    ...
}
public class Warrior: Player {
    ...
    public void Wield(Sword sword) { ... }
    ...
}

"Each weapon deals damage to the attacked enemy". Ok, now we have to have a common interface for Weapon:

interface Weapon {
    void dealDamageTo(Player enemy);
}

And so on... Why is there no Wield() in the Player? Because there was no requirement that any Player can wield any Weapon.

I can image, that there would be a requirement that says: "Any Player may try to wield any Weapon." This would be a completely different thing however. I would model it perhaps this way:

interface Player {
    void Attack(Player enemy);
    void TryWielding(Weapon weapon); // Throws UnwieldableException
}

Summary: Model the requirements, and only the requirements. Do not do data modeling, that is not oo modeling.

  • 1
    Did you read the series? Maybe you want to tell the author of that series not to model data but requirements. The rquirements you have in your answer are your made-up requirements NOT the requirements the author had when building the C# compiler. – CodingYoshi Nov 23 '17 at 12:59
  • 2
    Eric Lippert is detailing a technical problem in that series, which is fine. This question is about the actual problem however, not C# features. My point is, in real projects we are supposed to follow the business requirements (for which I gave made-up examples, yes), not assume relations and properties. That is how you get a model that fits. Which was the quesetion. – Robert Bräutigam Nov 23 '17 at 13:20
  • That's the first thing I thought when reading that series. The author just came up with some abstractions, never evaluating them further, just sticking with them. Trying to mechanically solve the problem, again and again. Instead of thinking about a domain and abstractions that are useful, that should apparently go first. My upvote. – Zapadlo Nov 24 '17 at 8:19
  • This is the correct answer. The article expresses conflicting requirements (one requirement says that a player can wield a [any] weapon, while other requirements say that this is not the case.) and then details how difficult it is for the system to correctly express the conflict. The only correct answer is to remove the conflict. In this case, that means remove the requirement that a player can wield any weapon. – Daniel T. Nov 26 '17 at 22:30
2

One way would be to pass the Wield command to the Player. The player then executes the Wield command, which checks the appropriate rules and returns the Weapon, which the Player then sets its own Weapon field with. This way, the Weapon field can have a private setter and is only settable via passing a Wield command to the player.

  • Actually this does not solve the problem. The developer who is making the command object may pass any weapon and the player will set it. Go read the series because the problem is more difficult than you think. Actually he made that series because he came across this design issue while developing the Roslyn C# compiler. – CodingYoshi Nov 23 '17 at 12:53
2

Nothing prevents the developer from doing that. Actually Eric Lippert tried many different techniques but they all had weaknesses. That was the whole point of that series that stopping the developer from doing so is not easy and everything he tried had disadvantages. Finally he decided that using a Command object with rules is the way to go.

With rules, you can set Weapon property of a Wizard to be a Sword but when you ask the Wizard to wield the weapon (Sword) and attack it will not have any effect and therefore it will not change any state. As he says below:

We have another rule that strengthens the first rule, that says that the first rule’s effects do not apply when a wizard tries to wield a sword. The effects for that situation are “make a sad trombone sound, the user loses their action for this turn, no game state is mutated

In another words, we cannot enforce such a rule through type relationships which he tried many different ways but either did not like it or did not work. Thus the only thing he said we can do is to do something about it at runtime. Throwing an exception was no good because he does not consider it an exception.

He finally chose to go with the above solution. This solution basically says you can set any weapon but when you yield it, if not the right weapon, it would be essentially useless. But no exception would be thrown.

I think its a good solution. Though in some cases I would go with the try-set pattern too.

  • This solution basically says you can set any weapon but when you yield it, if not the right weapon, it would be essentially useless. I failed to find it in that series, could you please point me where this solution is proposed? – Zapadlo Nov 24 '17 at 8:16
  • @zapadlo He says it indirectly. I have copied that part in my answer and quoted it. Here it is again. In the quote he says: when a wizard tries to wield a sword. How can a wizard wield a sword if a sword was not set? It must have been set. Then if a wizard wields a sword The effects for that situation are “make a sad trombone sound, the user loses their action for this turn – CodingYoshi Nov 24 '17 at 12:41
  • Hmm, I think wielding a swords basically means that it should be set, no? As I read that paragraph, I interpret it that the first rule's effect is that the existing weapon, if there is one, is dropped and the new weapon becomes the player’s weapon. While the second rule which that strengthens the first rule, that says that the first rule’s effects do not apply when a wizard tries to wield a sword. So I think that there is a rule checking if the weapon is sword, so it can not be wielded by a wizzard, so it is not set. Instead a sad trombone sounds. – Zapadlo Nov 24 '17 at 13:12
  • To my thinking it would be strange that some command rule gets violated. It's like blurring over a problem. Why handling this problem after a rule is already violated and setting a wizard into an invalid state? The wizard can not have a sword, but it does! Why not letting it happen? – Zapadlo Nov 24 '17 at 13:12
  • I agree with @Zapadlo on how to interpret Wield here. I think it is a slightly misleading name for the command. Something like ChangeWeapon would be more accurate. I guess you could have a different model where you can set any weapon but when you yield it, if not the right weapon, it would be essentially useless. That sounds interesting, but I don't think it is what Eric Lippert describes. – Ben L Nov 24 '17 at 14:16
2

The author's first discarded solution was to represent the rules by the type system. The type system is evaluated at compile time. If you detach the rules from the type system, they are not checked by the compiler anymore, so there is nothing that prevents a developer from making a mistake per se.

But this problem is faced by every piece of logic/modelling that is not checked by the compiler and the general answer to this is (unit) testing. Therefore, the author's proposed solution needs a strong test harness to circumvent errors by developers. To underline this point of needing a strong test harness for errors that only be detected at runtime, look at this article by Bruce Eckel, which makes an argument that you need exchange the strong typing for stronger testing in dynamic languages.

In conclusion, the only thing that can prevent developers from making mistakes is having a set of (unit) tests checking that all rules are adhered to.

  • You are to use an API and the API is supposed to ensure you will do unit testing or make that assumption? Th whole challenge is about modelling so the model does not break even if the developer using the model is careless. – CodingYoshi Nov 24 '17 at 6:15
  • 1
    The point I was trying to make was that there is nothing that prevents the developer from making mistakes. In the proposed solution, the rules are detached from the data, so if you don't create your own checks, there is nothing preventing you from using the data objects without applying the rules. – larsbe Nov 24 '17 at 9:52
1

I may have missed a subtlety here, but I'm not sure the problem is with the type system. Maybe it's with convention in C#.

For example, you can make this completely type safe by making the Weapon setter protected in Player. Then add setSword(Sword) and setStaff(Staff) to Warrior and Wizard respectively that call the protected setter.

That way, the Player/Weapon relationship is statically checked and code that doesn't care can just use a Player to get a Weapon.

  • Eric Lippert did not want to throw exceptions. Did you read the series? The solution has to meet the requirements and this requirements are stated clearly in the series. – CodingYoshi Nov 23 '17 at 12:55
  • @CodingYoshi Why would this throw an exception? It's type safe i.e. checkable at compile time. – Alex Nov 23 '17 at 12:58
  • Sorry couldn't change my comment once I realized you are not throwing excpetion. However, You broke the inheritance by doing that. See the issue the author was trying to solve is you cannot just add a method like you did because now the types cannot be treated polymorphically. – CodingYoshi Nov 23 '17 at 13:03
  • @CodingYoshi The polymorphic requirement is that a Player has a Weapon. And in this scheme, the Player indeed has a Weapon. No inheritance is broken. This solution will only compile if you get the rules right. – Alex Nov 23 '17 at 13:32
  • @CodingYoshi Now, that doesn't mean you can't write code that would require a runtime check e.g. if you try to add a Weapon to a Player. But there is no type system where you don't know the concrete types at compile time that can act on those concrete types at compile time. By definition. This scheme means that it is only that case that needs to be dealt with at runtime, As such is actually better than any of Eric's schemes. – Alex Nov 23 '17 at 13:36
0

So, what does prevent a developer from doing this? Do they just have to remember not to?

This question is effectively the same with pretty holy-war-ish topic called "where to put validation" (most probably noting ddd as well).

So, before answering this question, one should ask yourself: what is the nature of the rules that you want to follow? Are they carved in stone and define the entity? Does the breaking of those rules result in an entity to stop being what it is? If yes, along with keeping these rules in command validation, put them in an entity as well. So if a developer forgets to validate the command, your entities won't be in an invalid state.

If no -- well, it inherently implies that this rules are command specific and should not reside in domain entities. So violating this rules results in an actions that shouldn't have been allowed to take place, but not in invalid model state.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.