refactor python strategy pattern to use abstract base class

Question

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()

• hjpotter92: why not use ABC for characters?
• David J.: @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.
• hjpotter92: but you're defining the use_weapon for each subclass for weapon behaviour.
• David J.: @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?
• slepic: 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 :)

Answer 1

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()
```