5
\$\begingroup\$

My program represents a basic model of a player in Fantasy Football with players playing in different positions and having different scoring systems for the actions on the pitch.

A particular usage is a table with players and their values per price which provides some suggestions about who to pick into the team.

I would like to get feedback about my design choice to represent a player as a class and the way I used Python class model for doing that, particularly usage of inheritance.

I'm also wondering whether representing the scoring system as class level attributes for every position is the common way of doing things.

The code:

"""Basic modelling of a player in Fantasy Football via classes with inheritance."""

class Player():
    """Base class representing a player in Fantasy Football."""
    assist = 3
    pos_impact = 1
    neg_impact = -1
    yellow_card = -1
    made_start = 1
    played_60minutes = 1

    def __init__(self, name='', price=-1, club=''):
        """Initialises a player with some shared properties."""
        self.name = name
        self.price = price
        self.club = club
        self.assist = 0
        self.pos_impact = 0
        self.neg_impact = 0
        self.yellow_card = 0
        self.made_start = 1
        self.played_60minutes = 0.95

    def calculate_points(self):
        """Calculates points for properties shared by all the players."""
        total_points = 0
        total_points += self.assist * Player.assist
        total_points += self.pos_impact * Player.pos_impact + self.neg_impact * Player.neg_impact
        total_points += self.yellow_card * Player.yellow_card
        total_points += self.made_start * Player.made_start
        return total_points

    def calculate_value_per_price(self):
        """Calculates how many points a player earns per price."""
        total_points = self.calculate_points()
        return total_points / self.price

class Goalkeeper(Player):
    """Class representing a goalkeeper in fantasy football."""
    goal = 8
    clean_sheet = 4
    save = 0.5
    two_or_more_conceded = -1

    def __init__(self, name='', price=-1, club=''):
        """Initialises a goalkeeper."""
        super().__init__(name, price, club)
        self.goal = 0
        self.clean_sheet = 0
        self.save = 0
        self.two_or_more_conceded = 0

    def calculate_points(self):
        """Calculates points for a goalkeeper."""
        total_points = super().calculate_points()
        total_points += self.goal * Goalkeeper.goal
        total_points += self.clean_sheet * Goalkeeper.clean_sheet
        total_points += self.save * Goalkeeper.save
        total_points += self.two_or_more_conceded * Goalkeeper.two_or_more_conceded
        return total_points

class Defender(Player):
    """Class representing a defender in fantasy football."""
    goal = 6
    clean_sheet = 4
    two_or_more_conceded = -1

    def __init__(self, name='', price=-1, club=''):
        """Initialises a defender."""
        super().__init__(name, price, club)
        self.goal = 0
        self.clean_sheet = 0
        self.two_or_more_conceded = 0

    def calculate_points(self):
        """Calculates points for a defender."""
        total_points = super().calculate_points()
        total_points += self.goal * Defender.goal
        total_points += self.clean_sheet * Defender.clean_sheet
        total_points += self.two_or_more_conceded * Defender.two_or_more_conceded
        return total_points

class Midfielder(Player):
    """Class representing a midfielder in fantasy football."""
    goal = 5
    clean_sheet = 1
    played_90minutes = 1

    def __init__(self, name='', price=-1, club=''):
        """Initialises a midfielder."""
        super().__init__(name, price, club)
        self.goal = 0
        self.clean_sheet = 0
        self.played_90minutes = 0

    def calculate_points(self):
        """Calculates points for a midfielder."""
        total_points = super().calculate_points()
        total_points += self.goal * Midfielder.goal
        total_points += self.clean_sheet * Midfielder.clean_sheet
        total_points += self.played_90minutes * Midfielder.played_90minutes
        return total_points

class Forward(Player):
    """Class representing a forward in fantasy football."""
    goal = 4
    played_90minutes = 1

    def __init__(self, name='', price=-1, club=''):
        """Initialises a forward."""
        super().__init__(name, price, club)
        self.goal = 0
        self.played_90minutes = 0

    def calculate_points(self):
        """Calculates points for a forward."""
        total_points = super().calculate_points()
        total_points += self.goal * Forward.goal
        total_points += self.played_90minutes * Forward.played_90minutes
        return total_points

def compare_players(players):
    """Compares players by value per price."""
    print_players_comparison(calculate_players_comparison(players))

def calculate_players_comparison(players):
    """
    Creates a table with players properties;
    Sorted by value per price in descending order.
    """
    players_stats = []
    for player in players:
        player_stats = {}
        player_stats['name'] = player.name
        player_stats['club'] = player.club
        player_stats['points'] = player.calculate_points()
        player_stats['price'] = player.price
        player_stats['value_per_price'] = player.calculate_value_per_price()
        players_stats.append(player_stats)
    return sorted(players_stats, key=lambda x: x['value_per_price'], reverse=True)

def print_players_comparison(players_stats):
    """Prints players comparison."""
    print(f'{"Name":<16} {"Club":<16} {"Points":<10} {"Price":<10} {"Value per price":<10}')
    print('='*72)
    for player in players_stats:
        print(f'{player["name"]:<16} {player["club"]:<16} {player["points"]:<10.2f} {player["price"]:<10.2f} {player["value_per_price"]:<10.2f}')


if __name__ == '__main__':
    ederson = Goalkeeper(name='Ederson', price=11.5, club='Man City')
    reguilon = Defender(name='Sergio Reguilon', price=8.8, club='Tottenham')
    grealish = Midfielder(name='Jack Grealish', price=9.2, club='Aston Villa')
    maupay = Forward(name='Neal Maupay', price=7.6, club='Brighton')

    ederson.pos_impact = 0.65
    ederson.neg_impact = 0.20
    ederson.yellow_card = 0.05
    ederson.clean_sheet = 0.40
    ederson.save = 3
    ederson.two_or_more_conceded = 0.29

    reguilon.pos_impact = 0.52
    reguilon.neg_impact = 0.26
    reguilon.yellow_card = 0.10
    reguilon.assist = 0.08
    reguilon.goal = 0.03
    reguilon.clean_sheet = 0.36
    reguilon.two_or_more_conceded = 0.32

    grealish.pos_impact = 0.42
    grealish.neg_impact = 0.33
    grealish.yellow_card = 0.07
    grealish.played_90minutes = 0.80
    grealish.assist = 0.30
    grealish.goal = 0.35
    grealish.clean_sheet = 0.25

    maupay.pos_impact = 0.18
    maupay.neg_impact = 0.49
    maupay.yellow_card = 0.05
    maupay.played_90minutes = 0.65
    maupay.assist = 0.12
    maupay.goal = 0.26

    players = [ederson, reguilon, grealish, maupay]

    compare_players(players)
\$\endgroup\$
2
+100
\$\begingroup\$

Efficiency of multiplication

For low-throughput tasks - calling calculate_points ten or even one hundred times a second - what you have is fine. If you want to run high-throughput simulations (for instance), this:

assist = 3
pos_impact = 1
neg_impact = -1
yellow_card = -1
made_start = 1
played_60minutes = 1
# ...

    total_points = 0
    total_points += self.assist * Player.assist
    total_points += self.pos_impact * Player.pos_impact + self.neg_impact * Player.neg_impact
    total_points += self.yellow_card * Player.yellow_card
    total_points += self.made_start * Player.made_start

is well-modelled by a vector dot-product, which you can easily do via Numpy.

Even if you don't use Numpy, consider using a namedtuple:

Weights = namedtuple('Weights', (
    'assist', 
    'pos_impact',
    'neg_impact',
    'yellow_card',
    'made_start',
    'played_60minutes',
))

coefficients = Weights(3, 1, -1, -1, 1, 1)

# ...

total_points = sum(c*w for c, w in zip(coefficients, self.weights))

It seems that your calculate_points forgot to use played_60minutes; I don't know whether that was deliberate.

The same approach can be followed for Goalkeeper, etc.

Early serialization

You should rethink calculate_players_comparison and print_players_comparison. There is no reason to store this as a dictionary. Instead, consider

  • Making a secondary @dataclass class ComparisonData with typed fields of name: str, points: float, etc.
  • Including a @staticmethod def print_header()
  • Including a def print(self)
  • Returning an Iterable[ComparisonData] from calculate_players_comparison
  • Having print_players_comparison call into print_header() and then print() for each row from calculate_players_comparison

Suggested

You could get very fancy with a metaclass that auto-defines a tuple based on keyword-only arguments; something like:

class WeightedMeta(type):
    def __new__(mcs, name: str, bases: tuple, attrs: dict):
        sig = inspect.signature(attrs['__init__'])
        params = [p for p in sig.parameters.values()
                  if p.kind is p.KEYWORD_ONLY]
        names = [p.name for p in params]
        defaults = [p.default for p in params if p.default is not p.empty]
        point_type = namedtuple('point_type', names, defaults=defaults)
        names = set(names)
        old_init = attrs['__init__']

        def new_init(self, *args, **kwargs):
            self.points = point_type(**{
                k: v for k, v in kwargs.items()
                if k in names
            })
            old_init(self, *args, **kwargs)

        attrs.update(point_type=point_type, __init__=new_init)
        cls = super().__new__(mcs, name, bases, attrs)
        weights = cls.get_weights()

        @property
        def total_points(self) -> float:
            """Calculates points for properties shared by all the players."""
            return sum(w*p for w, p in zip(weights, self.points))

        cls.total_points = total_points
        return cls

But that's a little much, and it doesn't handle derivation well. An easier way to approach this is

"""
Basic modelling of a player in Fantasy Football via classes with inheritance.
"""
from dataclasses import dataclass, astuple, asdict
from sys import stdout
from typing import TextIO, Iterable


class Player:
    """Base class representing a player in Fantasy Football."""

    @dataclass
    class Points:
        assist: float = 0
        pos_impact: float = 0
        neg_impact: float = 0
        yellow_card: float = 0
        made_start: float = 1
        played_60minutes: float = 0.95

    weights = Points(
        assist=3,
        pos_impact=1,
        neg_impact=-1,
        yellow_card=-1,
        made_start=1,
        played_60minutes=1,
    )

    def __init__(
        self,
        name: str,
        price: float,
        club: str,
        points: Points
    ):
        self.name, self.price, self.club, self.points = name, price, club, points

    @property
    def total_points(self) -> float:
        return sum(
            w*p
            for w, p in zip(astuple(self.weights), astuple(self.points))
        )

    @property
    def value_per_price(self) -> float:
        """Calculates how many points a player earns per price."""
        return self.total_points / self.price

    def __str__(self):
        return self.name

    @staticmethod
    def print_table(players: Iterable['Player'], f: TextIO = stdout):
        print(
            '=' * 72 + '\n' +
            f'{"Name":<16} {"Club":<16} {"Points":>6} '
            f'{"Price":>10} {"Value per price":>15}',
            file=f,
        )
        for player in sorted(players, key=lambda p: p.value_per_price, reverse=True):
            player.print_row(f)

    def print_row(self, f: TextIO = stdout):
        """Prints players comparison."""
        print(
            f'{self.name:<16} {self.club:<16} {self.total_points:>6.2f} '
            f'{self.price:>10.2f} {self.value_per_price:>15.2f}',
            file=f,
        )


class Goalkeeper(Player):
    @dataclass
    class Points(Player.Points):
        goal: float = 0
        clean_sheet: float = 0
        save: float = 0
        two_or_more_conceded: float = 0

    weights = Points(
        goal=8,
        clean_sheet=4,
        save=0.5,
        two_or_more_conceded=-1,
        **asdict(Player.weights),
    )


class Defender(Player):
    @dataclass
    class Points(Player.Points):
        goal: float = 0
        clean_sheet: float = 0
        two_or_more_conceded: float = 0

    weights = Points(
        goal=6,
        clean_sheet=4,
        two_or_more_conceded=-1,
        **asdict(Player.weights),
    )


class Midfielder(Player):
    @dataclass
    class Points(Player.Points):
        goal: float = 0
        clean_sheet: float = 0
        played_90minutes: float = 0

    weights = Points(
        goal=5,
        clean_sheet=1,
        played_90minutes=1,
        **asdict(Player.weights),
    )


class Forward(Player):
    @dataclass
    class Points(Player.Points):
        goal: float = 0
        played_90minutes: float = 0

    weights = Points(
        goal=4,
        played_90minutes=1,
        **asdict(Player.weights),
    )


def main():
    players = [
        Goalkeeper(
            name='Ederson', price=11.5, club='Man City',
            points=Goalkeeper.Points(
                pos_impact=0.65,
                neg_impact=0.20,
                yellow_card=0.05,
                clean_sheet=0.40,
                save=3,
                two_or_more_conceded=0.29,
            ),
        ),

        Defender(
            name='Sergio Reguilon', price=8.8, club='Tottenham',
            points=Defender.Points(
                pos_impact=0.52,
                neg_impact=0.26,
                yellow_card=0.10,
                assist=0.08,
                goal=0.03,
                clean_sheet=0.36,
                two_or_more_conceded=0.32,
            ),
        ),

        Midfielder(
            name='Jack Grealish', price=9.2, club='Aston Villa',
            points=Midfielder.Points(
                pos_impact=0.42,
                neg_impact=0.33,
                yellow_card=0.07,
                played_90minutes=0.80,
                assist=0.30,
                goal=0.35,
                clean_sheet=0.25,
            ),
        ),

        Forward(
            name='Neal Maupay', price=7.6, club='Brighton',
            points=Forward.Points(
                pos_impact=0.18,
                neg_impact=0.49,
                yellow_card=0.05,
                played_90minutes=0.65,
                assist=0.12,
                goal=0.26,
            ),
        ),
    ]

    Player.print_table(players)


if __name__ == '__main__':
    main()

Simple, slightly-better-typed, and less repetitive.

\$\endgroup\$
6
  • \$\begingroup\$ Thanks for the review! As far as Weights are concerned, where should I define them? At the class level of every player position? Talking about early serialization, I'm neither familiar with @staticmethod nor with @dataclass, could you provide an example of coding player_comparison one or another way? \$\endgroup\$ Dec 16 '20 at 21:36
  • \$\begingroup\$ Seems like you have used self.weights in calculating total_points, which implies that it is defined as an attribute of an instance of a class, rather than class level attribute. I don't understand why every instance should have its own weights instead of one per class. \$\endgroup\$ Dec 16 '20 at 21:41
  • 1
    \$\begingroup\$ There would be one namedtuple definition per class, so the definition could live as a static. weights would indeed be an instance attribute. I'll edit to show an example. \$\endgroup\$
    – Reinderien
    Dec 16 '20 at 22:57
  • \$\begingroup\$ Scoring system is different for every position (class), rather than for every player (instance), so that it should be one per class. Having said that, why do you suggest making weights an instance attribute? \$\endgroup\$ Dec 16 '20 at 23:12
  • 1
    \$\begingroup\$ @KonstantinKostanzhoglo You'll see that in the example code I offer, weights is effectively the coefficients and points is the cost vector. \$\endgroup\$
    – Reinderien
    Dec 18 '20 at 4:07
3
\$\begingroup\$

It looks like the only difference between the classes is the weights used in calculating the points. The player classes may have different weights defined and/or the weights may have different values. Let the weights be defines by class variables with names starting with w_. __init__() used inspect.getmembers() to find all the weights and create a dict of corresponding attributes. Attribute values can be set when a class is instantiated or using the .update() method.

All the functionality is in the Player class. The specialized subclassed just add or change some weights.

Code revised to add initial values. Class vars starting with i_ are initial/default values for the corresponding attributes.

import inspect

class Player:
    w_assist = 3
    w_pos_impact = 1
    w_neg_impact = -1
    w_yellow_card = -1
    w_made_start = 1
    w_played_60minutes = 1
    
    i_made_start = 1
    i_played_60minutes = 0.95
    
    def __init__(self, name='', price=-1, club='', **kwargs):
        self.name = name
        self.price = price
        self.club = club
        self.points = 0
        self.value_ratio = 0
        
        self.attributes = {}
        for key, value in inspect.getmembers(self):
            if key.startswith('w_') and key[2:] not in self.attributes:
                self.attributes[key[2:]] = 0
                
            elif key.startswith('i_'):
                self.attributes[key[2:]] = value
        
        if kwargs:
            self.update(**kwargs)
            
    def update(self, **kwargs):
        for key, value in kwargs.items():
            if key in self.attributes:
                self.attributes[key] = value
            else:
                raise KeyError(f"{self.name} doesn't have an attribute '{key}'")
        
        self.points = sum(v*getattr(self, f'w_{k}') for k,v in self.attributes.items())
        self.value_ratio = self.points / self.price

class Goalkeeper(Player):
    """Class representing a goalkeeper in fantasy football."""
    w_goal = 8
    w_clean_sheet = 4
    w_save = 0.5
    w_two_or_more_conceded = -1
    
    

class Defender(Player):
    """Class representing a defender in fantasy football."""
    w_goal = 6
    w_clean_sheet = 4
    w_two_or_more_conceded = -1
    
    

class Midfielder(Player):
    """Class representing a midfielder in fantasy football."""
    w_goal = 5
    w_clean_sheet = 1
    w_played_90minutes = 1
    
    

class Forward(Player):
    """Class representing a forward in fantasy football."""
    w_goal = 4
    w_played_90minutes = 1
    

    
def compare_players(players):
    header_fmt = '{:<16} {:<16} {:<10} {:<10} {:<10}'.format
    table_fmt  = '{:<16} {:<16} {:<10.2f} {:<10.2f} {:<10.2f}'.format
    
    print(header_fmt("Name", "Club", "Points", "Price", "Value"))
    print('='*72)

    for player in sorted(players, key=lambda p:p.value_ratio, reverse=True):
        print(table_fmt(player.name, player.club, player.points, player.price, player.value_ratio))


if __name__ == "__main__":

    # set attributes in the constructor
    ederson = Goalkeeper(name='Ederson', price=11.5, club='Man City',
                         pos_impact=0.65,
                         neg_impact=0.20,
                         yellow_card=0.05,
                         clean_sheet=0.40,
                         save=3,
                         two_or_more_conceded=0.29
                        )
    
    # set attributes in .update()
    reguilon = Defender(name='Sergio Reguilon', price=8.8, club='Tottenham')
    reguilon.update(pos_impact=0.52,
                    neg_impact=0.26,
                    yellow_card=0.10,
                    assist=0.08,
                    goal=0.03,
                    clean_sheet=0.36,
                    two_or_more_conceded=0.32
                   )
    
    # set some attributes in the constructor and others in .update()
    grealish = Midfielder(name='Jack Grealish', price=9.2, club='Aston Villa',
                          played_90minutes=0.80,
                          assist=0.30,
                          goal=0.35,
                          clean_sheet=0.25
                         )
    grealish.update(pos_impact=0.42, neg_impact=0.33, yellow_card=0.07)
    

    maupay = Forward(name='Neal Maupay', price=7.6, club='Brighton',
                     pos_impact=0.18,
                     neg_impact=0.49,
                     yellow_card=0.05,
                     played_90minutes=0.65,
                     assist=0.12,
                     goal=0.26
                    )

    players = [ederson, reguilon, grealish, maupay]

    compare_players(players)

Output:

Name             Club             Points     Price      Value     
========================================================================
Jack Grealish    Aston Villa      5.67       9.20       0.62      
Neal Maupay      Brighton         3.64       7.60       0.48      
Ederson          Man City         5.16       11.50      0.45      
Sergio Reguilon  Tottenham        3.65       8.80       0.41      
\$\endgroup\$
7
  • \$\begingroup\$ Thanks for the review! Enjoyed your idea of using inspect and update. Just one thing: my code gets exactly 1 point more for each player, I'm wondering where you've lost it. \$\endgroup\$ Dec 18 '20 at 11:30
  • 1
    \$\begingroup\$ This code seems to lose the OP's ability to define defaults per point variable. \$\endgroup\$
    – Reinderien
    Dec 18 '20 at 14:50
  • \$\begingroup\$ @KonstantinKostanzhoglo, Possibly because I initialized all the attributes to zero, while you initialized self.made_start = 1 and self.played_60minutes = 0.95 in Player.__init__(). \$\endgroup\$
    – RootTwo
    Dec 18 '20 at 16:05
  • \$\begingroup\$ @Reinderien, that's true. Initial values could be added using i_... class vars and processing them in Player.__init__() similar to the w_... class vars. Or, have the w_... vars be a (weight, initial value) tuple and modify __init__() and the sum(...) call in update() accordingly. \$\endgroup\$
    – RootTwo
    Dec 18 '20 at 16:48
  • \$\begingroup\$ @RootTwo could you update the code to fix this? \$\endgroup\$ Dec 18 '20 at 16:49
0
\$\begingroup\$

First thing that comes to mind is, that you should consider looking into @dataclass decorator. I've never used it with inheritance, but I expect it can work and that would remove like 60% of your code, making it a lot cleaner :-)

I don't like those -1 initial values, that you reset in constructor anyway. If anything set them to None, so that at least code fails if it's unset rather than calculates with -1, I don't expect you'd want that.

\$\endgroup\$
3
  • \$\begingroup\$ Could you provide an example of using @dataclass? I use -1 initial value so that it would not blow up when calculating derived attributes in case the proper value wasn't provided when creating an object, which would do should you use None instead. \$\endgroup\$ Dec 14 '20 at 0:41
  • \$\begingroup\$ Why is that desired? When proper value wasn't provided, is object even valid? I'd expect not and in that case you want it to crash immediately rather than having incorrect results later. As per dataclass, probably decorating your classes and removing init methods should do the trick. Also then you can pass all data into constructor rather than setting after object was created. \$\endgroup\$
    – K.H.
    Dec 14 '20 at 0:47
  • \$\begingroup\$ In my case the value isn't provided at all, rather than proper value isn't provided. It is still valid to calculate player's points, which is useful. Anyways, in the very basic version it doesn't matter because the possible use cases are not known in advance. As far as dataclass is concerned: how you tried doing it? Since I haven't uesd this feature, it is not clear how to do that from the description above. \$\endgroup\$ Dec 14 '20 at 0:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.