Dynamic class instancing (with conditional parameters and methods) based on a dictionary

In an RPG game I'm developing, after building a base Weapon class, instead of having many subclasses of Weapon, I want the weapons to be built upon dictionary parameters. Many of them will be simple static information (weight, value, etc.), but others will have conditionally defined parameters, with not only numeric/textual values: they may have to call external methods, such as spell-like effect, feat, skill, combat maneuvers, etc.).

In this example I'm using strings, like "unholy", to represent what will become method calls (with a given name) at some point.

The number of items (that will share most of this Weapon example's design, once I'm done with it), eventually, should reach the hundreds (the PRD rules I'm adapting to this game already provide a few hundred "basic items"), and allow the player to build new ones, both at runtime based on existing models (crafting) and pre-runtime (modding).

One weapon, for example, should change its stats and associated functions (spell-like effects) when equipped according to the wielder's class. But many more different conditions should be needed/used as I add fancy magic weapons, armors and items.

By managing to add new weapons without having to mess with the class itself, but instead with a dictionary/JSON (once the core code is built, hopefully with some great advice from this community), the user would benefit from accessible code and even the developer would gain in productivity by handling only dictionaries and relying on an existing core logic.

This currently relies on a few setattr and eval calls to handle strings from the dictionary. I would like to hear suggestions on preferable approaches, concerning performance, security, etc.. Can it be significantly improved somehow? Readability (always) matters.

```
# A design test for dynamic class building.
# > attributes and conditional logic are readed from a dictionary.

# This dictionary will actually reside in another file, maybe a json or
# gzipped p

Nice code! Clean, and pretty Pythonic IMO. Without further ado...

General stuff

# This dictionary will actually reside in another file, maybe a json or
# gzipped pickled json...
WEAPONS = {
}

The first time I read this post, I was like "this guy is out of his mind" for the ridiculous nesting abuse of the dictionary syntax. Now I understand :)

item.owner = self

This is interesting to me. item is already privately initialised for a given instance; why the redundancy?
It reads well, but I don't understand why it makes sense.

def is_of_class(self, _class):
  return self._class == _class

Nitpick: Python already has two ways to check if something is a subclass / instance of something.
I think the name of this method should be is_instanceof or so -- I know I'd be more likely to remember that, anyways. (Even if it contains a string not a Python object, I say the same.)

Code duplication

Let's take a look at this:

def equip(self):
    """Set item status and call its on equip functions."""
    self.equipped = True
    for action in self.on_equip:
        if action['type'] == "check":
            self.check(action)
        elif action['type'] == "action":
            self.action(action)

def unequip(self):
    """Unset item dynamic status, call its on unequip functions."""
    self.equipped = False
    for action in self.on_unequip:
        if action['type'] == "check":
            self.check(action)
        elif action['type'] == "action":
            self.action(action)

# --------------

def check(self, dic):
    """Check a condition and call an action according to it."""
    check_act = eval(dic['condition'][0])
    args = dic['condition'][1]
    result = check_act(*args)

    self.action(*dic[result])

def action(self, *dicts):
    """Perform action with args, both specified on dicts."""
    for dic in dicts:
        act = eval(dic['action'])
        act(dic['args'])

# --------------

def add_on_hit(self, actions):
    for action in actions:
        if action not in self.on_hit_actions:
            self.on_hit_actions.append(action)

def add_on_turn(self, actions):
    for action in actions:
        if action not in self.on_turn_actions:
            self.on_turn_actions.append(action)

# --------------

def rem_on_hit(self, actions):
    for action in actions:
        try:
            self.on_hit_actions.remove(action)
        except ValueError:
            pass

def rem_on_turn(self, actions):
    for action in actions:
        try:
            self.on_turn_actions.remove(action)
        except ValueError:
            pass

Notice anything... macro? To me, what stands out is that each of the methods in the groups shares a literally identical shape to the eye when glancing over it, and upon closer inspection they each do the exact same thing in some different semantic context.

Whenever I come across cases of this all-but-semantic-duplication in my own code, my solution (typically) is to write one method that takes arguments of do.what, to.what, how, etc. Generally, I think code that actually does something good and minimises duplication, even if that one method does get lengthy, is overall better than duplicated methods.

I could go into my spiel about why I think code deduplication is important, but you've probably heard it before and if you haven't, you can just google "why code duplication is evil" or so.

Q & A With your friendly neighbourhood static analysis tool: pylint

pylint3 has a lot of miscellaneous comments to make about your code, most of which I ignore because they are really noise when we Humans know what the code actually does.

However, pylint did bring up some good points in the discussion we had. A synopsis:

W:109,20: Use of eval (eval-used)
W:111,17: Used * or ** magic (star-args)
W:118,18: Use of eval (eval-used)
R:115, 4: Method could be a function (no-self-use)

Okay, interesting. Let's talk about these!

Eval.

`

Eval... is cool. It's interesting, it's the fun you get to have with an interpreted language.

Having said that, it's evil in almost every case, and I would argue that this is possibly one of those cases. For instance, were this game MMORPG, I can put whatever junk I want into a JSON doc, pickle it, hand it to this code, and boom: your game logic's hacked. I can even put arbitrary code execution in, let alone arbitrary data!

Eval. is. evil.

On a more serious note, I would recommend against eval for real implementations in all cases unless your code, and only strings generated within Python's memory space, are what's being eval'd.



"Magical" stars

Actually, pylint, get with the times, because * and **` unzipping is totally awesome and there's no reason to avoid it. Sometimes, static analysis is, well, static. Like the kind on the radio :)

"Method could be a function"

Yeah, the first time I saw this it took me a minute too. What it's saying is that this method could be refactored into a func

StackExchange Code Review Q#118687, answer score: 3