Design: method in class or general function?

[Leam Hall]

On 09/08/2017 03:06 AM, Peter Otten wrote:
> leam hall wrote:
> 
>> On Thu, Sep 7, 2017 at 8:16 AM, Steve D'Aprano
>> <steve+python at pearwood.info> wrote:
>>
>>> On Thu, 7 Sep 2017 07:20 pm, Leam Hall wrote:
>>>
>>>> OOP newbie on Python 2.6.
>>>
>>> Python 2.6 is ancient, and is missing many nice features. You should
>>> consider
>>> using the latest version, 3.6.
>>>
>>
>> I've wrestled with that discussion for a while and Python 3 loses every
>> time. There's literally no good reason for me to move to Python 3 earlier
>> than mid-2020's. Please accept the fact that there are hundreds of
>> thousands of servers, if not millions, running Python 2.x. Whether or not
>> Python 3 has any neat cool stuff is irrelevant to those of us seeking to
>> use Python to get today's work done.
>>
>>> I create instances of Character class with an attribute dict of
>>>> 'skills'. The 'skills' dict has the name of a skill as the key and an
>>>> int as a value. The code adds or modifies skills before outputting the
>>>> Character.
>>>>
>>>> Is it better design to have a Character.method that takes a 'skill' key
>>>> and optional value or to have a general function that takes an
>>>> instance, a dict, a key, and an optional value?
>>>
>>> I'm afraid your example is too generic for me to give an opinion. Do you
>>> literally mean a method called "method"? What does it do?
>>>
>>
>>
>> Using this:
>> https://github.com/makhidkarun/py_tools/blob/master/lib/character.py
>>
>> Line 19 sets "self.skills" either from the passed in data or from
>> https://github.com/makhidkarun/py_tools/blob/master/lib/character_tools.py
>> #L34-L48
>>
>> So Character.skills is a dict with a string key and an int value. I need
>> to be able to add skills and my first attempt is a function:
>> https://github.com/makhidkarun/py_tools/blob/master/lib/character_tools.py
>> #L52-L56
>>
>> Should the "add_skills" function be a method in the character class or be
>> made a more generic function to add/modify a key/value pair in a dict that
>> is an attribute of an instance? Other tasks will require the add/modify
>> functionality but coding that increases complexity. At least for me,
>> anyway.
>>
>> Sorry about being unclear earlier, coffee was still kicking in and I'm
>> still a newbie that mixes up terms.
> 
> I'm pleading "method" as it allows per-class implementation.
> 
> Say you use per-career subclasses of a general Character class. There are
> default per-career skill sets, but usually a Character can acquire a skill
> that is not needed in his career -- with the exception that Rogues cannot
> tap dance ;)
> 
> Below is a way to implement that with a specialised add_skill() method:
> 
> $ cat basic_oo.py
> from __future__ import print_function
> import random
> from collections import defaultdict
> 
> 
> class Character(object):
>      DEFAULT_SKILLS = ['Blade', 'GunCbt', 'Admin', 'Streetwise']
> 
>      def __init__(self):
>          self.skills = defaultdict(int)
> 
>      def add_random_skills(self, terms):
>          skillnames = self.DEFAULT_SKILLS
>          for _ in range(2*terms):
>              self.add_skill(random.choice(skillnames))
> 
>      def add_skill(self, name, amount=1):
>          self.skills[name] += amount
> 
>      def __str__(self):
>          skills = ", ".join(
>              "{}={}".format(name, amount)
>              for name, amount in sorted(self.skills.items())
>              if amount != 0
>          )
>          return "{}({})".format(self.__class__.__name__, skills)
> 
> 
> class Rogue(Character):
>      def add_skill(self, name, amount=1):
>          if name == "TapDance":
>              raise ValueError("Sorry, this rogue will never tap dance")
>          super(Rogue, self).add_skill(name, amount)
> 
> 
> class Marine(Character):
>      DEFAULT_SKILLS = ['GunCbt', 'VaccSuit', 'Leadership', 'Vehicle']
> 
> 
> def main():
>      NUM_CHARACTERS = 5
>      CHARACTERS = [Marine, Rogue]
> 
>      characters = [
>          random.choice(CHARACTERS)() for _ in range(NUM_CHARACTERS)
>      ]
> 
>      for c in characters:
>          c.add_random_skills(5)
>          c.add_skill("RepairBicycles", random.randrange(3))
>          try:
>              c.add_skill("TapDance", 3)
>          except ValueError as err:
>              print(err)
> 
>      for c in characters:
>          print(c)
> 
> 
> if __name__ == "__main__":
>      main()
> 
> $ python basic_oo.py
> Sorry, this rogue will never tap dance
> Sorry, this rogue will never tap dance
> Sorry, this rogue will never tap dance
> Rogue(Admin=3, Blade=4, GunCbt=2, Streetwise=1)
> Marine(GunCbt=5, Leadership=4, TapDance=3, VaccSuit=1)
> Rogue(Blade=3, GunCbt=2, RepairBicycles=2, Streetwise=5)
> Rogue(Admin=1, Blade=2, GunCbt=5, RepairBicycles=1, Streetwise=2)
> Marine(GunCbt=1, Leadership=3, RepairBicycles=2, TapDance=3, VaccSuit=2,
> Vehicle=4)


Okay Peter, I took your idea and mangled it beyond recognition. There's 
a design constraint I hadn't mentioned: an instance of Character should 
be able to have multiple careers.

Also, an instance can be created from scratch, created from a full set 
of data, or created from a partial set of data.

Here's my current code, focusing on creating a lot of merchants: 
https://github.com/makhidkarun/py_tools/blob/merchant/lib/character.py#L60-L61

python chargen.py
Toby Verstraeten     564775     [M] Age: 41
   Merchant (5 terms)
   Computer-2 Engineering-5 Navigation-2 Pilot-1

Captain Ian Domici           789987     [M] Age: 24
   Firster (3 terms) Merchant (3 terms) Noble (2 terms)
   Broker-2 GunCbt-1 Streetwise-2

Rosa                 66495B     [F] Age: 24
   Merchant (1 terms)
   Computer-1 Navigation-1


As you can see, lots to work on. However, with a very loose definition 
of "work", this works.

The goal is to have a set of Career classes. The program will allow a 
user to select one or more Careers. The program will create a basic 
character and then modify the character by the selected Career class(es).

If the Career does not exist in a file then the character gets assigned 
a generic Career based on social status.

Careers are each in their own file and the program builds the list of 
available careers by slurping up those file names.
   Adding a Career should just be adding a properly formatted file.
   Import happens when a Career is selected to modify the character

I've done this in Ruby so my guess is it can be done in Python. Even 
Python 2.  :D

The Career seems to be a "Decorator" pattern given my limited 
understanding of design patterns. Concur? If so I'll go study that some 
more.

Do you feel this path should still make a Career a class?

Thanks!

Leam