3
\$\begingroup\$

I came across this strategy pattern implementation

https://github.com/jtortorelli/head-first-design-patterns-python/blob/master/src/python/chapter_1/adventure_game.py

class Character:
    def __init__(self):
        self.weapon_behavior = None

    def set_weapon(self, weapon_behavior):
        self.weapon_behavior = weapon_behavior

    def fight(self):
        self.weapon_behavior.use_weapon()


class Queen(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = KnifeBehavior()


class King(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = BowAndArrowBehavior()


class Troll(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = AxeBehavior()


class Knight(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = SwordBehavior()


class WeaponBehavior:
    def use_weapon(self):
        raise NotImplementedError


class KnifeBehavior(WeaponBehavior):
    def use_weapon(self):
        print("Stabby stab stab")


class BowAndArrowBehavior(WeaponBehavior):
    def use_weapon(self):
        print("Thwing!")


class AxeBehavior(WeaponBehavior):
    def use_weapon(self):
        print("Whack!")


class SwordBehavior(WeaponBehavior):
    def use_weapon(self):
        print("Thrust!")


knight = Knight()
king = King()
queen = Queen()
troll = Troll()
knight.fight()
king.fight()
queen.fight()
troll.fight()

Would it be correct to refactor it the following way, using an ABC?

from abc import ABC, abstractmethod


class Character:
    def __init__(self):
        self.weapon_behavior = None

    def set_weapon(self, weapon_behavior):
        self.weapon_behavior = weapon_behavior

    def fight(self):
        self.weapon_behavior.use_weapon()


class Queen(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = KnifeBehavior()


class King(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = BowAndArrowBehavior()


class Troll(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = AxeBehavior()


class Knight(Character):
    def __init__(self):
        super().__init__()
        self.weapon_behavior = SwordBehavior()


class WeaponBehavior(ABC):
    @abstractmethod
    def use_weapon(self, message):
        print(message)

class KnifeBehavior(WeaponBehavior):

    def use_weapon(self):
        super().use_weapon("Stabby stab stab")


class BowAndArrowBehavior(WeaponBehavior):
    def use_weapon(self):
        super().use_weapon("Thwing!")

class AxeBehavior(WeaponBehavior):
    def use_weapon(self):
        super().use_weapon("Whack!")


class SwordBehavior(WeaponBehavior):
    def use_weapon(self):
        super().use_weapon("Thrust!")


knight = Knight()
king = King()
queen = Queen()
troll = Troll()
knight.fight()
king.fight()
queen.fight()
troll.fight()
\$\endgroup\$
6
  • \$\begingroup\$ why not use ABC for characters? \$\endgroup\$
    – hjpotter92
    Commented Oct 7, 2020 at 6:50
  • \$\begingroup\$ @hjpotter92 I'm not entirely sure, I felt it wouldn't be ideal because each subclass of character doesn't and shouldn't define its own method for fight and set_weapon. I'm new to ABCs though, so I could be wrong. \$\endgroup\$
    – David J.
    Commented Oct 7, 2020 at 7:14
  • \$\begingroup\$ but you're defining the use_weapon for each subclass for weapon behaviour. \$\endgroup\$
    – hjpotter92
    Commented Oct 7, 2020 at 7:28
  • \$\begingroup\$ @hjpotter92 yeah, that's intentional - each definitoion of use_weapon is unique to the subclass. each definition of fight and set_weapon wouldn't be unique. Or am I missing something? \$\endgroup\$
    – David J.
    Commented Oct 7, 2020 at 7:34
  • \$\begingroup\$ Only the Character and WeaponBehaviour classes are actually useful and neither of them needs to inherit anything. The rest can be just factory functions, because only the constructors differ. If a class constructor does anything except assign arguments to properties, it is probably wrong. And btw I just learnt that behavior is prefered in american english and behaviour everywhere else :) \$\endgroup\$
    – slepic
    Commented Oct 7, 2020 at 14:34

1 Answer 1

4
\$\begingroup\$

Only the Character and WeaponBehaviour classes are actually useful and neither of them needs to inherit anything. The rest can be just factory functions, because only the constructors differ. If a class constructor does anything except assign arguments to properties, it is probably wrong.

Strategy pattern is based on composition rather then inheritance.

class Character:
    def __init__(self, weapon):
        self.weapon = weapon

    def set_weapon(self, weapon):
        self.weapon = weapon

    def fight(self):
        self.weapon.use()


def Queen():
    return Character(Knife())

def King():
    return Character(Bow())

class Weapon:
    def __init__(self, message):
       self.message = message
    def use(self):
       print(self.message)


def Knife():
    return Weapon("Stabby stab stab")

def Bow():
    return Weapon("Thwing!")


king = King()
queen = Queen()
king.fight()
queen.fight()
queen.set_weapon(Bow())
queen.fight()

Notice that I have removed the behavior part of the names as it seemed a bit useless.

I have also renamed use_weapon() to just use() because it is called on a weapon variable, and so it seemed redundant.

Also notice, that unless I used the set_weapon() method on a constructed Character instance (ie. to switch weapon in middle of battle), the Character class would be useless, because everything could have been done with the weapons alone. Of course I know this is just a pattern demonstration code, but I wanted to point out anyway..

As a bonus, here is something (not only) for the king :) Also notice how again composition is prefered over inheritance to provide flexibility.

class DoubleWeapon:
    def __init__(self, left, right):
        self.left = left
        self.right = right
    def use(self):
        self.left.use()
        self.right.use()

king.set_weapon(DoubleWeapon(Knife(), Sword()))
king.fight()
```
\$\endgroup\$

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.