How to refactor multiple big if statements?

Question

I have a coroutine in Python which takes a number of objects, determines the property of each one of them and then based on those results loads a correct resource. The problem is, a variable number of objects can be supplied (1, 2, 3..), each of which has 1 of the N available states {A, B, C, D, E...}. So, we can have an interaction of type A, B, C... for 1 object, interaction of type AB, AA, BA... (order matters, repetitions are allowed) for 2 objects and so on for 3 or more. The naive solution (mine) is to check how many objects we have and then YandereDev style check every possible combination:

@client.command()
async def hug(ctx, members: commands.Greedy[discord.Member]):
    count = len(members)
    if count == 1:
        if ctx.author.id == members[0].id:
            author = await get_roles_single(ctx.author)
            if author == 'Male':
                os.chdir(base + '\\hug\\SM')
            elif author == 'Female':
                os.chdir(base + '\\hug\\SF')
            (file, embed) = await prepare_embed("Self Hug", f"{ctx.author.mention} hugs themselves.", 'hug.gif', ctx.author.color, False)
            await ctx.send(file=file, embed=embed)
            return
        author = await get_roles_single(ctx.author)
        recepient = await get_roles_single(members[0])
        if author == recepient == 'Male':
            os.chdir(base + '\\hug\\men')
        elif author == recepient == 'Female':
            os.chdir(base + '\\hug\\women')
        elif (author, recepient) in (('Male', 'Female'), ('Male', 'Female')):
            os.chdir(base + '\\hug\\mix')
        else:
            os.chdir(base + '\\hug\\default')
        (file, embed) = await prepare_embed("Hug", f"{ctx.author.mention} hugs {members[0].mention}.", 'hug.gif', ctx.author.color, False)
        await ctx.send(file=file, embed=embed)
        os.chdir(base)
    elif count == 2:
        pass

This coroutine is incomplete, I omitted the case for 2 people, but it follows the same logic, although it's even more convoluted. (My apologies)

As to explain, what's going on there, this coroutine is a discord command, where a member mentions another member and some action happens based on their gender roles. If it matters, multiple gender roles are supported, not just male of female (although these have been omitted in this example).

My question is, how exactly do I refactor the ifs, while we also consider that we can have a member ping 1, 2, 3 or more members with order needing to stay preserved. How do we factor out this huge conditional logic?

Answer 1

We can simplify this portion :

    if count == 1:
        if ctx.author.id == members[0].id:
            author = await get_roles_single(ctx.author)
            if author == 'Male':
                os.chdir(base + '\\hug\\SM')
            elif author == 'Female':
                os.chdir(base + '\\hug\\SF')
            (file, embed) = await prepare_embed("Self Hug", f"{ctx.author.mention} hugs themselves.", 'hug.gif', ctx.author.color, False)
            await ctx.send(file=file, embed=embed)
            return
        author = await get_roles_single(ctx.author)
        recepient = await get_roles_single(members[0])

with :

    if count == 1:
        author = await get_roles_single(ctx.author)
        if ctx.author.id == members[0].id:
            os.chdir(base + '\\hug\\S'+ author[0])
            (file, embed) = await prepare_embed("Self Hug", f"{ctx.author.mention} hugs themselves.", 'hug.gif', ctx.author.color, False)
            await ctx.send(file=file, embed=embed)
            return
        # author = await get_roles_single(ctx.author)
        recipient = await get_roles_single(members[0])

And for this portion:

        if author == recepient == 'Male':
            os.chdir(base + '\\hug\\men')
        elif author == recepient == 'Female':
            os.chdir(base + '\\hug\\women')
        elif (author, recepient) in (('Male', 'Female'), ('Male', 'Female')):
            os.chdir(base + '\\hug\\mix')
        else:
            os.chdir(base + '\\hug\\default')

with:

        _dict = {'MM' : 'men', 'FF' : 'women', 'MF' : 'mix', 'FM' : 'mix'}
        _key  = author[0] + recipient[0]
        os.chdir(base + '\\hug\\' + _dict.get(_key, 'default'))

Answer 2

You could first construct a string, representing the exact interaction happening, consisting of unique symbols. For example, something like this:

# "Am" = Author, male. "Mf" = Member, female...
interaction = ""
for member in members:
    if ctx.author.id == member.id:
        interaction += "A"
    else:
        interaction += "M"

    role = await get_roles_single(member)
    if role == 'Male':
        interaction += "m"
    elif.... # other roles

return interaction # e.g. "AmMfMf"

Now we have the exact interaction. If every possible interaction requires a unique treatment (that's how I understand it) you can define commands for every interaction and map them to the interaction strings. As commands you can use functions, lambda function or classes. For example:

# just as an example, what will be called
def selfhug_male(author, members):
    os.chdir(base + '\\hug\\SM')
    (file, embed) = await prepare_embed("Self Hug", f"{author.mention} hugs themselves.", 'hug.gif', author.color, False)
    await ctx.send(file=file, embed=embed)

# all the commands
commands = {
    "AmAm": selfhug_male,
    "AfAf": selfhug_female,
}

#usage (I ignored concurrency here, you might have to modify):
command = commands[interaction]
command(author, members)

You might need to write a separate command for every interaction, but probably they can be reused or cleverly constructed, as I suspect some system in the treatment of the interactions.

Answer 3

For 2 Objects of 2 available states (A, B):

interaction of A -> B:  A's reaction of B
interaction of B -> A:  B's reaction of A
interaction of A -> A:  A's reaction of another A
interaction of B -> B:  B's reaction of another B

If above example with different response per interaction is ultimate goal, but without if chaining - Create each State as class and assign users with States class. Dispatching interaction target's via target's State class would make expansion much easier.

To expand type of States, just create more State classes and register interactions in each classes for every type of States you have.

If you want to expand Number of Objects interacting altogether beyond 2, you'll have to modify or add new Decorator to register / dispatch with more arguments accordingly.

Here's basic dispatching logic - which defines responses for each State without single if branching. Will get above interaction results for demonstration.

from functools import wraps


def state_deco(func_main):
    """
    Decorator that mimics singledispatch for ease of interaction expansions.
    """

    # assuming no args are needed for interaction functions.
    func_main.dispatch_list = {}  # collect decorated functions

    @wraps(func_main)
    def wrapper(target):
        # dispatch target to destination interaction function.
        nonlocal func_main
        try:
            # find and run callable for target
            return func_main.dispatch_list[type(target)]()
        except KeyError:
            # If no matching case found, main decorated function will run instead.
            func_main()

    def register(target):
        # A decorator that register decorated function to main decorated function.
        def decorate(func_sub):
            nonlocal func_main
            func_main.dispatch_list[target] = func_sub

            def register_wrapper(*args, **kwargs):
                return func_sub(*args, **kwargs)

            return register_wrapper
        return decorate

    wrapper.register = register
    return wrapper


# Abstract class of reactions
class StateBase:
    # Implement per states
    def interaction(self, target) -> str:
        raise NotImplementedError


class StateA(StateBase):
    def interaction(self, target):
        # if interaction target is not registered, general() will run instead.

        @state_deco
        def general():
            # Add some necessary setups like os.chdir here, and below functions.
            # return is not needed, just for demonstration.
            return "A's reaction to undefined others."

        @general.register(StateA)
        def _():
            # Function name is not required, up to you whether name it or not.
            return "A's reaction of another A"

        @general.register(StateB)
        def _():
            return "A's reaction of B"

        return general(target)


class StateB(StateBase):
    def interaction(self, target):

        @state_deco
        def general():
            return "B's reaction to undefined others."

        @general.register(StateA)
        def _():
            return "B's reaction of A"

        @general.register(StateB)
        def _():
            return "B's reaction of another B"

        return general(target)

# Expand States responses further for more interactions choices.


if __name__ == '__main__':
    # pretending users got their roles via get_roles_single()
    user_A = StateA()
    user_B = StateB()

    print(f"interaction of A -> B:  {user_A.interaction(user_B)}")
    print(f"interaction of B -> A:  {user_B.interaction(user_A)}")
    print(f"interaction of A -> A:  {user_A.interaction(user_A)}")
    print(f"interaction of B -> B:  {user_B.interaction(user_B)}")

Answer 4

Well it's a bit hard to grasp the whole logic without the generalization for count > 1, but a few thoughts:

• split the logic in as many functions as possible. For example, a get_path_for_members which would return the path to move to according to the genders.

• try to find a general rule if possible, instead of having special cases all over the place (one logic for any number of members for ex).

• if you still have to handle special cases separately, move each into a separate function, and factor out whatever is in common (like get_roles_single?)

author = await get_roles_single(ctx.author)
roles = [await get_roles_single(m) for m in members]
rel_path = get_path_for_members(author, roles)    # "\\hug\\SM", "\\hug\\women" etc
title, msg = get_message(author, roles)    # "Hug", ".. hugs .." or "Self Hug", ".. hugs themselves"
os.chdir(base + rel_path)
(file, embed) = await prepare_embed(title, msg, 'hug.gif', ctx.author.color, False)
await ctx.send(file=file, embed=embed)
os.chdir(base)

Answer 5

Did you mean:

for member in members:
    if ctx.author.id == member.id:
        ...

ie replace members[0] with member?