Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OverusedExpressionViolation and TooManyExpressionsViolation #1077

Closed
AlwxSin opened this issue Dec 17, 2019 · 10 comments
Closed

OverusedExpressionViolation and TooManyExpressionsViolation #1077

AlwxSin opened this issue Dec 17, 2019 · 10 comments
Labels
bug Something isn't working documentation Docs related task help wanted Extra attention is needed level:starter Good for newcomers

Comments

@AlwxSin
Copy link
Contributor

AlwxSin commented Dec 17, 2019

Bug report

What's wrong

What's the difference between these violations? And how they work together?
Also, it's hard to understand options, how should I combine them?

max-expressions=9 # TooManyExpressionsViolation for module, function or method
max-module-expressions=7 # OverusedExpressionViolation for module
max-function-expressions=4 # OverusedExpressionViolation for function or method

How is that should be

Looks like TooManyExpressionsViolation doesn't need at all.

@AlwxSin AlwxSin added the bug Something isn't working label Dec 17, 2019
@sobolevn
Copy link
Member

sobolevn commented Dec 17, 2019

Oh, I see.

I guess we have to update the docs for this one. TooManyExpressionsViolation is about any expressions, like:

def test():
    print(1)
    print(2)
    call_other(var)
    # ...

These are 3 expressions. We don't care about their semantics in this violation.
And we can limit how many expressions there should be in a single function. That's why we use this violation.

But, OverusedExpressionViolation is about overusing one particular expression in your source code (based on function and module counts):

def test1():
    print(my[key])

def test2():
    if my[key] > 1:
        call(my[key])

In this snippet we have my[key] used three times: once per first function, twice in the second one, three times in the whole module. We can limit how many times one can use my[key] per function or module.

Does it make it sense to you?

@sobolevn sobolevn added the documentation Docs related task label Dec 17, 2019
@sobolevn sobolevn added this to the Version 0.13.x milestone Dec 17, 2019
@sobolevn sobolevn added help wanted Extra attention is needed level:starter Good for newcomers labels Dec 17, 2019
@AlwxSin
Copy link
Contributor Author

AlwxSin commented Dec 17, 2019

It does make sense. How about specifying that overuse of same expression for OverusedExpressionViolation?

@sobolevn
Copy link
Member

Yeap 👍

And I guess we can also link them together (with py:class:). Because they are quite similar and users might want to read more about the differences and similarities between them.

@sobolevn sobolevn mentioned this issue Dec 18, 2019
4 tasks
@ManishAradwad
Copy link
Contributor

I got these violations but I didn't get why do we need them...
I mean what could go wrong if we use too many expressions in a function or use single function too many times??

@sobolevn
Copy link
Member

sobolevn commented Dec 18, 2019

Your functions will grow too big. And you will fail to understand them.
Smaller functions = better composition and better readability.
See: https://sobolevn.me/2019/10/complexity-waterfall

Expressions overuse on the other hand indicates, that you are repeating some important thing without proper abstraction. That's how my_list[0][1] becomes username = and obj.find('<button>', 1) becomes obj.find_button()

@sobolevn
Copy link
Member

@AlwxSin do you want to fix the docs for this one?

@ko3luhbka
Copy link
Contributor

Hi!
Could you please explain how not to violate WPS204 rule? In my tests I often used this pattern:

response = client.get(some_url)
page_content = response.content.decode('utf-8')
...some content checks

I faced with WPS204, and yes, there are a lot of repeated response.content.decode('utf-8') expressions. So I moved the expression into function like this:

def decode_response(response):
    return response.content.decode('utf-8')

... and call it in each test.
But I still have a problem with WPS204:
Found overused expression: decode_response(response); used 12 > 7
Am I missing something? How to properly refactor this?
Thanks in advance!

@AlwxSin
Copy link
Contributor Author

AlwxSin commented Jun 9, 2020

@ko3luhbka just turn off this rule in tests.

@ko3luhbka
Copy link
Contributor

@ko3luhbka just turn off this rule in tests.

Oh, okay, it was a kind of "last resort" for me. I just thought that it's possible to fix it "right" somehow.
Thank you.

@sobolevn
Copy link
Member

sobolevn commented Jun 9, 2020

The "right" way would be supported in #1120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Docs related task help wanted Extra attention is needed level:starter Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants