
I review code for a living and these are my most common review comments - ygkhatri
https://medium.com/@surajarya/i-review-code-for-a-living-and-these-are-my-most-common-review-comments-4c178f135c4a
======
stevesycombacct
One bit of feedback on "3\. Mutating values"

The reason that

    
    
        _ = [validations.pop(key) for key in ('executed','filter_type')]
         

is a mutation is because of the .pop() command, which the article didn't
state. .pop() removes items in-place, thus mutating the data. Since the
function doesn't make a copy of the dictionary, the original variable gets
mutated.

This may confuse those wondering what the difference between the two functions
is.

Otherwise, I find the article's points reasonable.

------
thdc
I'm not sure if I agree with #5 and #8.

For #5, more early returns means you don't have to keep track of as many paths
as you go through the code. Granted you could argue your function could
warrant splitting up if you end up with too many returns.

#8 is dependent on whether or not it's safe to continue if one execution of
the loop body fails (although he does say don't do it if you're going for that
behavior, which I'd like to believe is the more common option)

------
nunez
Nice article!

