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

try-else clauses measured at 1 more than correct complexity #27

Open
bukzor opened this issue Aug 11, 2014 · 5 comments
Open

try-else clauses measured at 1 more than correct complexity #27

bukzor opened this issue Aug 11, 2014 · 5 comments

Comments

@bukzor
Copy link
Contributor

bukzor commented Aug 11, 2014

Currently the below code generates the following graph, giving a complexity of 5:

def f():
    try:
        print(1)
    except TypeA:
        print(2)
    except TypeB:
        print(3)
    else:
        print(4)
    finally:
        if x:
            print(5.1)
        else:
            print(5.2)

mccabe

I don't think this models the try-else clause correctly. The above graph seems to indicate that the else sometimes runs instead of the main clause, which isn't correct. I believe this is the correct model:

mccabe

This seems to correctly indicate that the else always runs directly after the main try clause, if it runs, and never in a line of execution that involves the exception handlers. This has 1 less complexity than the above graph. This yields one less complexity than the current interpretation.

@bukzor
Copy link
Contributor Author

bukzor commented Aug 11, 2014

Also see discussion on #26

@sigmavirus24
Copy link
Member

Sorry for the confusion on #26. I agree that this is the right graph (with the exception of where the edges from the try body start for the except clauses.

@bukzor
Copy link
Contributor Author

bukzor commented Aug 12, 2014

Should we get more buy-in before I submit a patch?
The quick hack I put in to make that diagram is pretty ugly.

@bukzor bukzor changed the title How should try-else clauses be modeled? try-else clauses measured at 1 more than correct complexity Aug 14, 2014
@zaneb
Copy link

zaneb commented Dec 18, 2017

Note that this problem is not at all restricted to try-except constructs with an else clause. Where there is no 'else' the graph simply contains an edge going directly from the 'try' statement to the end of the block without passing through either the body of the try or any of the exception handlers.

This is a regression in 0.3 (and all later versions) - 0.2.1 generates the correct graph. I suspect but have not bothered to verify that this is due to 0a42f06

i.e. this function:

def test_try(s):
    try:
        return int(s)
    except TypeError:
        return -1

returns a cyclomatic complexity of 3 instead of 2 (consistent with 0.2.1, but only because of the off-by-one error #5 that was fixed); and this function:

def test_try2(s1, s2):
    try:
        i1 = int(s1)
    except TypeError:
        i1 = -1
    try:
        i2 = int(s2)
    except TypeError:
        i2 = -1
    return i1, i2

returns a cyclomatic complexity of 5 instead of 3 (vs. 4 in 0.2.1 due to the aforementioned off-by-one error fix.)

Ergo functions whose complexity is dominated by try/except have values that are off by up to a factor of 2.

@GerardSoleCa
Copy link

Note that this problem is not at all restricted to try-except constructs with an else clause. Where there is no 'else' the graph simply contains an edge going directly from the 'try' statement to the end of the block without passing through either the body of the try or any of the exception handlers.

This is a regression in 0.3 (and all later versions) - 0.2.1 generates the correct graph. I suspect but have not bothered to verify that this is due to 0a42f06

i.e. this function:

def test_try(s):
    try:
        return int(s)
    except TypeError:
        return -1

returns a cyclomatic complexity of 3 instead of 2 (consistent with 0.2.1, but only because of the off-by-one error #5 that was fixed); and this function:

def test_try2(s1, s2):
    try:
        i1 = int(s1)
    except TypeError:
        i1 = -1
    try:
        i2 = int(s2)
    except TypeError:
        i2 = -1
    return i1, i2

returns a cyclomatic complexity of 5 instead of 3 (vs. 4 in 0.2.1 due to the aforementioned off-by-one error fix.)

Ergo functions whose complexity is dominated by try/except have values that are off by up to a factor of 2.

Yes, we are experiencing the same issue, and honestly, to me seems a bit weird. Good I saw the same thing, it was driving me nuts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants