Open Stack

On Mon, Dec 8, 2014 at 5:03 PM, Brant Knudson <blk at acm.org> wrote:

>
> Not too long ago projects added a maximum complexity check to tox.ini, for
> example keystone has "max-complexity=24". Seemed like a good idea at the
> time, but in a recent attempt to lower the maximum complexity check in
> keystone[1][2], I found that the maximum complexity check can actually lead
> to less understandable code. This is because the check includes an embedded
> function's "complexity" in the function that it's in.
>

This behavior is expected.

Nested functions cannot be unit tested on there own.  Part of the issue is
that nested functions can access variables scoped to the outer function, so
the following function is valid:

 def outer():
    var = "outer"
    def inner():
        print var
    inner()


Because nested functions cannot easily be unit tested, and can be harder to
reason about since they can access variables that are part of the outer
function, I don't think they are easier to understand (there are still
cases where a nested function makes sense though).


> The way I would have lowered the complexity of the function in keystone is
> to extract the complex part into a new function. This can make the existing
> function much easier to understand for all the reasons that one defines a
> function for code. Since this new function is obviously only called from
> the function it's currently in, it makes sense to keep the new function
> inside the existing function. It's simpler to think about an embedded
> function because then you know it's only called from one place. The problem
> is, because of the existing complexity check behavior, this doesn't lower
> the "complexity" according to the complexity check, so you wind up putting
> the function as a new top-level, and now a reader is has to assume that the
> function could be called from anywhere and has to be much more cautious
> about changes to the function.
>

> Since the complexity check can lead to code that's harder to understand,
> it must be considered harmful and should be removed, at least until the
> incorrect behavior is corrected.
>

Why do you think the max complexity check is harmful? because it prevents
large amounts of nested functions?



>
> [1] https://review.openstack.org/#/c/139835/
> [2] https://review.openstack.org/#/c/139836/
> [3] https://review.openstack.org/#/c/140188/
>
> - Brant
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141209/35f13710/attachment.html>