@shadow_0771 asked for feedback on his code while learning OOP

@shadow_0771 asked for feedback on his code while learning OOP.

Well then... #code_roast!

RPG in the console is a classic. The code does its job, but it's written in a way that any attempt to scale it will cause MAAAAAJOR pain. Let's break down the most important issues 👇

1️⃣ Ctrl+C, Ctrl+V inheritance
Look at the armor hierarchy:

class Armor(Item):
    def __init__(self, name, category, strength=None, value_strength=None...): # and another 100500 arguments
        super().__init__(name, category)
        # ...

class Helmet(Armor):
    def __init__(self, ...):
        super().__init__(...)

class Chestplate(Armor):
    # Copy of Helmet

class Greaves(Armor):
    # Copy of Chestplate

All these classes (Helmet, Chestplate, Greaves, Boots) are absolutely identical. They don't add any new attributes or behavior. They do nothing but call super().__init__.

OOP was not created to describe every physical object in the world with a separate class. If entities differ only by category name, it should be a single class Armor with an attribute slot_type (ideally via Enum).

2️⃣ Frankenstein constructor
Look at how an item is created:

crown = Helmet('Шлем Господства', 'Шлем', 'Сила', 5, 'Ловкость', 7, 'Интеллект', 3)

Never hardcode stat names into the method signature. Use dictionaries.
Instead of this mess of parameters, an item should accept stats={'strength': 5, 'agility': 7, 'intellect': 3}.

3️⃣ Class incest
The Characteristic class takes a hero and then does this:

for item in self._hero.slots_equipment.values():
    if item:
        if hasattr(item, 'value_strength') and item.value_strength:
            self.attributes['strength'] += item.value_strength

This is called "Tight Coupling". The characteristics class reaches into the hero's inventory with dirty hands, checks if there are items, and then via hasattr (which itself is a crutch in 99% of cases) tries to extract stats from them.

The hero should query his own equipment and pass the final modifiers to the characteristics system. Right now, the tail is wagging the dog.

4️⃣ Using Exceptions for logic
In the equip_armor method we see this:

try:
    if key not in self.slots_equipment:
        print('Нет такого слота.')
    # ... logic ...
except KeyError:
    print(f'Предмет не найден')

First, catching a broad KeyError will mask real bugs in your code (e.g., a typo in a dictionary inside the try block). Second, exceptions are for exceptional situations, not for checking if an item exists in inventory. For that, use the dictionary method .get().

In short, for a start, instead of 10 useless armor classes and monstrous constructors, you could at least do this:

from dataclasses import dataclass
from enum import Enum

class EquipmentSlot(Enum):
    HEAD = "Шлем"
    CHEST = "Нагрудник"
    WEAPON = "Оружие"

@dataclass
class Equipment:
    name: str
    slot: EquipmentSlot
    stats_bonus: dict[str, int]

# Creating an item:
crown = Equipment(
    name='Шлем Господства', 
    slot=EquipmentSlot.HEAD, 
    stats_bonus={'strength': 5, 'agility': 7, 'intellect': 3}
)

And that's it. No code duplication, protection against typos in slots via Enum, and an extensible stats system where you can add "Luck" tomorrow without rewriting __init__ in a dozen classes.

OOP is not when you have a separate class for every entity in the universe. OOP is about managing complexity and state.

But for one month of learning, this is an absolutely normal stage of evolution.

📖 Read:
- When a class in Python is evil: 6 cases where you complicate your life
- SOLID principles in OOP with examples in Python

Comments