pwoolvett / flake8_boolean_trap

A flake8 plugin to detect boolean traps.
10 stars 1 forks source link

Add example to README #2

Closed spaceone closed 1 year ago

spaceone commented 1 year ago

Hello,

I don't understand from reading the descriptions what the plugin does? Can you give one example and a explanation why one would not want to do it?

pwoolvett commented 1 year ago

https://www.youtube.com/watch?v=CnRkXO_a5mI

tl:dr: stuff like flip(True) are ambiguous. ¿Does it mean do not flip? ¿Does it mean flip in a reverse order?¿what is the default? etc. This plugin lints three conditions mentioned in the readme.

pwoolvett commented 1 year ago

contributions are welcome!

dwt commented 1 year ago

I too would like at least a link to some examples and why they are bad, or when they are not.

Example, today I had a function like this where it complained:

def replay_response(self, mark_as_replay=False):
    ...

This seems fine to me. The response is generated by default without the mark, but you can call it as replay_response(mark_as_replay=True) to override this behavior.

Now that I write this, the fix should probably be to mark it as a keyword only argument - if that is what this checker intends, that should go into the documentation though? If so, would this be a good example?

pwoolvett commented 1 year ago

Exactly.

That function definition would ALLOW usage such as this:


obj.replay_response(True)

The FBT002 you're getting means that method would be fine with that. (if you further add a type hint you'd also get FBT001).

So, the hint you get is:

$ flake8 i2.py 
i2.py:1:42: FBT002 do not set boolean defaults for positional args. Hint: in `def replay_response(...)`, define `mark_as_replay` as kw-only

Hint: in def replay_response(...), define mark_as_replay as kw-only

meaning you should change it to something like this:

def replay_response(self, *, mark_as_replay=False):
    ...
allburov commented 1 year ago

I love an article about that https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/

@pwoolvett shall I add both the article above (for those who like reading materials) and the video you suggested (for those who like video expalanations) to README.md?

pwoolvett commented 1 year ago

Sure! :)