CODEWITHSUNDEEP
python
gfdb
gfdb

Reputation: 319

All optional parameters but one must be passed

I am writing a class method that retrieves the player on a team. A player object has two unique attributes: account_id, account_name. The simplified class structure looks like this:

class Player:
    def __init__(self, account_name: str, account_id: str):
        self.account_name = account_name
        self.account_id = account_id

class Team:
    def __init__(self, players: List[Player]):
        self.players = players

(The team class has more than just a list of players as an attribute, I excluded them for simplicity)

I want to write a function Team.get_player() that has two optional parameters (account_id and account_name). I want the user of this function to be able to pass one or the other since you can find a player using either of them.

THE QUESTION: Does it make sense to have a function (similar to below) where there are two optional parameters, but the function requires that you pass one. Should I just divide the function into get_player_by_name, and get_player_by_account_id? I like the idea of having one function to do both but I'm unsure of the best way to do it or if it should even be done at all...

    def get_player(account_name: str = None,
                   account_id: str = None) -> Player:
        passed_args = locals()
        if not all(passed_args[key] is None for key in passed_args):
            raise TypeError('Team.get_player() takes 1 positional argument but 0 were given')
        if account_name:
            ...
        if account_id:
            ...

Upvotes: 0

Views: 825

Answers (1)

chepner
chepner

Reputation: 531185

Neither argument is optional on its own, but the use of one prohibits the use of the other: they are mutually exclusive.

It's cleaner to provide two separate functions, each of which has one required argument and is implemented in terms of a lower-level private method.

NameOrID = Union[Literal['account_name'], Literal['account_id']]


class Team:
    def __init__(self, players: List[Player]):
        self.players = players

    def get_player_by_name(self, name: str):
        return self._get_player('account_name', name)

    def get_player_by_id(self, id: str):
        return self._get_player('account_id', id)

    def _get_player(self, attr: NameOrId, val: str):
        for p in self.players:
            if getattr(p, attr) == val:
                return p

If you really want, you can make _get_player public, but it's not clear you need that kind of flexibility in your public API.

Upvotes: 1

Related Questions