lilserf / bot-on-the-clocktower

Bot to assist with playing Blood on the Clocktower via Discord
MIT License
22 stars 4 forks source link

No need of else statement #53

Closed AliHaider20 closed 3 years ago

AliHaider20 commented 3 years ago

class botcBot -> function getTownInfo. There's no need to else statement there.

Explanation: If the IF statement returns True it will return TownInfo(ctx.guild, doc) and jumps out of the function else the interpreter will go to the next line and then it will return None and jump out of the function.

magroader commented 3 years ago

Existing code:

        if doc:
            return TownInfo(ctx.guild, doc)
        else:
            return None

Suggestion:

        if doc:
            return TownInfo(ctx.guild, doc)
        return None

For that matter, the code could be:

        return doc and TownInfo(ctx.guild, doc) or None
AliHaider20 commented 3 years ago

@magroader Can you explain return doc and TownInfo(ctx.guild, doc) or None

magroader commented 3 years ago

Actually reading your comment again, I think you are saying that the return None is unnecessary because leaving a function in python will automatically return None. But I don't think we want to do that; we'd want to be explicit.

For an explanation, I am pretty sure this is how it works... it does in lua, and I think python acts the same way.

doc and TownInfo(ctx.guild, doc) if doc is not None, the and will trigger and move on to the TownInfo() portion - the statement will evaluate to a TownInfo object. It will not hit the or portion because the first portion is not None so it will short-circuit evaluating the or. if doc is None, it will skip the and and move on to the or instead, which will evaluate to None.

AliHaider20 commented 3 years ago

I'm not saying to remove the return None statement. What is this return doc and TownInfo(ctx.guild, doc) or None shorthand method called in python. Thanks for the explaination.

magroader commented 3 years ago

As mentioned in #54 , you are welcome to submit Pull Requests to help clean up the code, no overhead of making a new Issue required! If you put "Fixes #issuenum" in your Pull Request description, it'll auto-link them.

I believe the name of this shorthand is called "ShortHand Ternary" (https://book.pythontips.com/en/latest/ternary_operators.html)

AliHaider20 commented 3 years ago

Thanks a lot. Can you also explain how to add labels i.e tags to issues. I googled it much but can't understand. I'm not getting any option on RHS when I open any issue.

magroader commented 3 years ago

Ah, yeah I know that feeling, I've hit that before as well. Labels can only be added by people with certain permissions in the project repository, so only lilserf and I can do it.

If you'd like to contribute to Bot on the Clocktower or have questions, please make a change and submit a Pull Request or toss me an email (link in my profile). If you find any functionality bugs or have feature requests, please continue to submit issues! I'll close this one though as it's just a style suggestion that could be done via Pull Request.