Pythonic code review (Red Hat Security Blog)

[Posted December 15, 2016 by jake]

Over at the Red Hat Security Blog, Ilya Etingof writes about code reviews, in general, along with some specific thoughts on Pythonic versus non-Pythonic idioms in code. "People coming from Java tend to turn everything into a class. That's probably because Java heavily enforces the OOP paradigm. Python programmers enjoy a freedom of picking a programming model that is best suited for the task. The choice of object-based implementations look reasonable to me when there is a clear abstraction for the task being solved. Statefulness and duck-typed objects are another strong reason for going the OOP way. If the author's priority is to keep related functions together, pushing them to a class is an option to consider. Such classes may never need instantiation, though. Free-standing functions are easy to grasp, concise and light. When a function does not cause side-effects, it's also good for functional programming."

Pythonic code review (Red Hat Security Blog)

Posted Dec 15, 2016 21:50 UTC (Thu) by markhb (guest, #1003) [Link] (7 responses)

It seems he wrote far more on "interesting ways Python does things" than he did on more general insights into reviewing code. Also, Java does the exact same foreach loop he says it doesn't:


for (Object o : Collection<Object>) 

      //do stuff with o

}

Pythonic code review (Red Hat Security Blog)

Posted Dec 15, 2016 23:52 UTC (Thu) by lordsutch (guest, #53) [Link] (5 responses)

Isn't that for statement syntax a relatively new addition to Java?

Besides which, he specifically prefaces the section discussing for loops with "For folks coming from C."

Pythonic code review (Red Hat Security Blog)

Posted Dec 16, 2016 0:07 UTC (Fri) by JanC_ (guest, #34940) [Link]

"for folks coming from C" sounds like an interesting syntax proposal... ;-)

Java for-each

Posted Dec 16, 2016 0:26 UTC (Fri) by tialaramex (subscriber, #21167) [Link] (3 responses)

Java 5, so, about 12 years old at this point. It's no ANSI C, but it's past the point where anybody should be pretending that it's some bleeding edge technology they can't justify in their Real Grown-up systems.

Java for-each

Posted Dec 16, 2016 2:24 UTC (Fri) by smckay (guest, #103253) [Link]

And long since EOL. Any non-horrifying deployment will be running Java 7 or 8.

Java for-each

Posted Dec 16, 2016 12:20 UTC (Fri) by jtaylor (subscriber, #91739) [Link] (1 responses)

Even though it is available since a long time, there are lots of programmers that learned the old way and continue using it today.
I still regularly have to regularly flag stuff like C89 un-scoped iteration counters in >= C99 code reviews despite this being standard since 17 years and available even longer in actual compilers.

Java for-each

Posted Dec 20, 2016 18:34 UTC (Tue) by niner (subscriber, #26151) [Link]

Even longer in actual compilers....except for MSVC which finally started supporting C99 in 2012 and still doesn't have full support. So people who are used to cross platform development have learned to stick to C89 for a long time.

Pythonic code review (Red Hat Security Blog)

Posted Dec 16, 2016 15:37 UTC (Fri) by khim (subscriber, #9252) [Link]

I don't think he says that Java does not have a "foreach" loop. Of course it does! Who does not? Even C++ have a foreach loop novadays!

What he says is that Python only have a foreach loop! It does not include a traditional C/Java for loop! And he's right, of course.

Pythonic code review (Red Hat Security Blog)

Posted Dec 18, 2016 22:17 UTC (Sun) by ssmith32 (subscriber, #72404) [Link]

I've seen adverserial code reviews, and prolly even gotten overly defensive many moons ago (now I'm too busy and over it). But in a reasonably healthy environment they're great for many reasons beyond those listed.

Senior to Senior - we generally view the other person as doing as a favor, like "please god did I do the right thing in this legacy core business code? Because we really need this fixed, but I have no idea. Yeah? Basically ok, but change this or that? Cool. One less thing to worry me".

Junior reviewing senior code. Train them on the process on reviewing and maintaining, (so I don't have maintain so many things). Train them in an environment where the other engineer isn't going to be overly defensive. Get a feeling for how they think. Challenge them a little to see if they've thought things through, and if they have, they are right, and now the own the decison and what comes with it.

Consistency. There are a million little style ticks that one can have. Sometimes you just need to pick one, and stick with it. Review helps me with that, when submitting PRs against another style, and I just slip up and fall into habits, leaving an inconsistent style.