Frank Allan Harrison

Learning and teaching through code review

Elevating your teams to the highest common denominator.
4 min read28 Nov 2012by Frank

I've talked a few times about what makes good code review, and how it can help reduce the "bus-factor". Here is another core consideration I currently have whilst currently working at a place with a huge bus-factor[^1].

Code-review makes better developers and engineers.

Not only does code-review help your teams learn the ins and outs of your codebase(s), as intractable or perfect as it/they may be, it also helps elevate your teams to the highest-common-denominator of skill and knowledge.

Through code review, the worst developers are pulled closer to the orbit of the best developers, exposed to their knowledge, and the best developers collaborate to improve each other.

I am a self-taught programmer, I migrated from scriptkiddie to developer when I taught myself C++ with Stroustrup's "The C++ Programming Language", by reading it cover-to-cover. It was such a well written book and took you from the fundamentals through to some pretty serious techniques, at least back then. I remember how exciting it was to read, eventually understand, and then be able to implement stuff. Around the same time I taught myself ASM, compiler theory, joining the dots from my early BASIC days to "modern" programming. But, I wasn't very knuthian[^2]; I didn't use or think about algorithms or patterns much, I didn't think about my mental models, and I was constantly reinventing solutions and solving problems often working in my own little vacuum, pretty much... until, that is, I started working on teams where we used code-review.

In the code reviews I would often get questions like "why don't you just use algorithm X?", not knowing about X; or I would get statements like "cool, you're using pattern Y! That makes sense actually" when I wasn't actually aware of pattern Y! Or I would see a patch swapping out type A for type Z, for example std::vector with FixedArray or some other hand-rolled std:: drop-in; or some esoteric (to me) use of alorithm.h.

I would then have to go off, sometimes reluctantly, because I was young and arrogant, to look up the type, the algorithm or the pattern, and discover a whole new world, and sometimes a whole new way of thinking. For example, in the world of game-dev, stdlib was disallowed because back then stdlib was considered too slow/generic, which blew my mind because, at that time, I perceived it as being perfect, god-written, inviolable, infallible code. So, I quickly learned to evaluate everything, and do so quickly, thinking for myself instead of through received knowledge. I then learned how we, at FRD[^3], would role our own stdlib implementations; or, based on some profiling, we would swap out type A for type B with better packing and save x MBs; or through using Macros be able to optionally pre-compile game-levels to memory-ready binaries at compile-time, shaving 10s of seconds of level load-times. As an aside the above is exactly why Crytek bought FRD, we were very able to understand the performance needs of the CryEngine and make it run on consoles, elevating it, optimising it, improving it.

Those code reviews, that collaboration, was my apprenticeship. Those code reviews and discussions were where I really honed my trade-skills, and where I had a chance to return the favour, sharing with others the skills and tricks I had learned or worked out for myself.

To this day I still learn a lot from the other devs I work with, including the junior ones; new ways of thinking, new innovations I haven't come across yet, innovative ways to use old patterns, and more. In code reviews and design chats, I constantly discover new python modules, or C++ standard implementations, or books, or articles, or other fascinating things that help me become better, and I share them in kind with the people who's code I review, or who's designed I evaluate.

I probably learn more in a few high quality code reviews than I ever did doing my MSc.

[^1]: the risk resulting from information and capabilities not being shared among team members, derived from the phrase "in case they get hit by a bus"

[^2]: Donald Knuth is one of my heroes, his Christmas Lectures are a staple in my coding diet and The Art of Computer Programming books are incredible

[^3]: Free Radical Design, the last independent UK game studio