2

Before I ask my question, I'm aware of Eric Lippert's Wizard and Warrior series.

I'm trying to understand GRASP, but I'm having a hard time determining if this class violates GRASP.

Suppose if I had the following Character class:

class Character(ABC): def __init__(self, name, strength, attributes): self._name = name self._strength = strength self._attributes = attributes self._inventory = [] self._found_attributes = [] self._cannot_equip = False self._equipped = True def add(self, game_object): self._inventory.append(game_object) # I may obviously want to do more checks, such as required strength, # if I currently have another weapon equipped, etc.. def try_equip(self, game_object): if self._check_for_conflicting_attributes(game_object): return self._cannot_equip return self._equipped def _check_for_conflicting_attributes(self, game_object): for attrib in self._attributes: if game_object.contains_attribute(attrib): self._found_attributes.append(attrib) return len(self._found_attributes) > 0

Weapon class:

class GameWeapon(ABC): # imports and other methods left out. def __init__(self, name, required_strength, damage, attributes): self._name = name self._required_strength = required_strength self._damage = damage self._attributes : List[Attributes] = attributes def contains_attribute(self, attribute): return attribute in self._attributes

and Main:

def main(): wizard_attributes : List[Attributes] = [] wizard_attributes.append(Attributes.LongBlade) wizard_attributes.append(Attributes.Reloadable) wiz = Wizard("Gandalf", 100, wizard_attributes) sword_attributes : List[Attributes] = [] sword_attributes.append(Attributes.LongBlade) sword = Sword("Standard Sword", 5, 10, sword_attributes) # Will print false print(wiz.try_equip(sword)) if __name__ == "__main__": main()

Explanation:

Both Wizard and Sword have attributes, for example, Wizard might have attribute called mana-drinker and Sword might have LongBlade as an attribute. Before adding a LongBlade weapon to my character inventory, I check for specific attributes, for example, a Wizard can't use a Sword, so I check for LongBlade and if the weapon has that attribute it will prevent that weapon from being added to the character inventory.

One topic that's always mentioned is class responsibility, if the Character class does violate GRASP is it because it has the responsibility of checking and verifying if it could add a weapon to the player inventory?

My larger question is, how do you know when a class may be violating GRASP? What if I want the class or subclass to be responsible for check before adding a weapon or doing another task? Is it still a violation?

Improve this question
4
  • Your GRASP link is dead. Do you have another? A normal web page is preferable to a slideshow. Commented Feb 6, 2019 at 15:22
  • @JimmyJames - It's to a presentation, it works Commented Feb 6, 2019 at 15:23
  • 1
    Right, presentations are not a good for this purpose. People aren't going want to sit through a presentation to understand your question. Most people won't spend more than a minute or two looking at it. The wikipedia link in the answer below would be preferable. Commented Feb 6, 2019 at 15:28
  • 1
    @JimmyJames - I'll change the link Commented Feb 6, 2019 at 15:50

1 Answer 1

Reset to default
2

Answer to the immediate example

GRASP includes the pattern Information Expert, which states that whatever object contains the information required to perform an action is responsible for performing it.

In your example, your Sword does not contain sufficient information to determine whether a character can equip it or not. This information resides in the Character class. Therefore, it should be the Character's responsibility to perform the action.

To answer the broader question

Whether your objects violate GRASP differs depending on which pattern it violates. A violation of the Information Expert pattern mentioned above, for example, can be recognized because you find your method doing more calls for data from another object than it does consulting its own; there is not enough information to perform your task so you do an unreasonable amount of reach to gather that information.

When I apply GRASP to a design, I follow a sort of checklist.

  • Do the methods and members all make sense together (have high cohesion) or should they be divided into multiple smaller, more specialized components?
  • Can cohesion be improved or operations simplified by extracting functionality from one object to another which has a greater amount of the data required for the operation (Information expert)
  • How tightly knit are objects with other objects? Are they too tightly coupled with their dependencies, such that one cannot exist without the other, changing one irrevocably means I have to change the other and are their reuse too limited as a result of either of these cases.
  • What is the relationship between an object and any other object it might instantiate. Does instantiation follow the creator pattern rules.
  • Do I have operations that depend on conditional statements based on some attribute on otherwise interchangeable components? Can I abstract these components in order to avoid branching? (Polymorphism)
  • Does any of my objects contain members such that I cannot extend the functionality of the contained member without altering functionality on the object containing it? (Protected variation)
  • Can I reduce coupling between any objects by introducing an intermediary object? (Indirection)
  • Do my data-model classes contain input handling or otherwise allow data-models to be directly modified without an intermediary controller?
  • Is there any problem in my domain that can be simplified by the introduction of an intermediary that is not itself part of the domain? (Pure fabrication)

As you may notice, almost every bulletin on that checklist relates to interaction between objects, and that's because GRASP does not exist in isolation. In general, it does not make sense to ask if an individual object violates GRASP, because with only one object you only have one locus of responsibility. It's when you connect objects to one another that the overall design of the system may violate GRASP.

Edit

To clarify, the Information Expert pattern does not state that an object can never request information from another object, but rather that the object that needs to ask for the least amount of information should be responsible for the task.

Given the situation in the question, Character contains a rule stating it cannot equip Weapons with the LongBlade attribute. Therefore, all we need to do to find out if the Character is allowed to equip a weapon, is ask for the Weapons attribute contains LongBlade.

If we reverse this responsibility, and instead give Weapon the responsibility of deciding without changing anything else, the game asks the Character to equip a Weapon, the Character asks the Weapon if it can be equipped, and the Weapon asks the same Character if it is allowed to be equipped. The decision ultimately then remains in Character, and it should have the responsibility to make it.

Improve this answer
12
  • Does the Rule class suggestion presented by Eric Lippert violate GRASP? The Rule class will ask objects for information and decide what to do. Is the Rule class an example of a Pure Fabrication as discussed in the presentation? Commented Feb 6, 2019 at 14:41
  • Therefore, it should be the Character's responsibility to perform the action/you find your method doing more calls for data from another object than it does consulting its own - Bare with me here, but my Character class is not asking for information(calling a getter), it's passing the information into Sword and let Sword give you the result. I've updated my code, does it still violating GRASP. I added the Weapon class, didn't change thing else. Commented Feb 6, 2019 at 14:51
  • 1
    Additionally, going back a bit, the proposed Rule class could be a pure fabrication, if by omitting it, there is an increase in system complexity or coupling, or a reduction in cohesion. Commented Feb 6, 2019 at 15:35
  • 1
    I apologize, it seems I'm being unclear. What I am trying to say is that no, by having your Character ask Weapon for its type and then making a decision based on that, you are not in violation of GRASP. Character is the information expert in regards to what weapon types are valid, and as such given a weapon it can make the decision based on it. Therefore Character should have the responsibility - like it does in your example. You have correctly applied the Information Expert pattern, since Character does not defer its decision to another class. Commented Feb 6, 2019 at 15:44
  • 2
    @EmilyScott: correct, to extend the metaphor a bit, perhaps the Character is a weapons master and can use almost any weapon or almost anything as a weapon, only that particular character would know what could be used. Eric’s Rules wouldn’t cover it, because there wouldn’t be a rule for that (except to go through the characters skills and see, but even that would fail for can use non-weapon x as a weapon, as that wouldn’t be an enumerated skill but a result determined at runtime) and so Rules would have to ask the character anyway. Commented Feb 7, 2019 at 3:05