Code Review, Revision, and Technical Debt (PyCaribbean 2016)

Understanding how people learn to write can help us understand how people learn to code. Understanding how people learn to code can help us understand how to provide effective mentorship through code review. Understanding how people learn to code can also give us insight into how technical debt develops, and how to address it.

(Includes speaker notes)

Transcript

1. Code Review, Revision, and Technical Debt
2. Geoff Gerrietts @ggerrietts Development Manager @AppNeta 15 years of Python Really big nerd Geoff Gerrietts Geoff Gerrietts @ggerrietts @ggerrietts • Geoff Gerrietts • Development Manager at AppNeta • Pythonista for 15 years; developer for 20 • Manager, devops, programmer, webmaster, technical writer • Before that I studied English • I like to joke about how writing poetry and programming are similar.
3. Learning to Write • Now it gets real. • As a field of study, English offers a lot of specializations. • Most involve a lot of writing. • If you go far enough, they involve teaching people to write.
4. Geoff Gerrietts @ggerrietts Direct instruction produces limited results • One of the things you learn when you study the teaching of writing • Is that direct instruction produces limited results
5. Geoff Gerrietts @ggerrietts 1. Choose a thesis and write a topic sentence. 2. Support the topic sentence with ~3 supporting sentences. 3. Write a conclusion that summarizes what you have shown. • Probably everyone’s seen a little bit of this direct instruction. • A set of rules, which, when strictly followed, produce a functional result. • The result is rarely good, but it is functional.
6. Geoff Gerrietts @ggerrietts “The best writing breaks the rules, but you need to learn the rules to break the rules.” • It’s an open secret that the best prose never conforms to the rules. • Teachers know it, and they say infuriating things like this.
7. Geoff Gerrietts @ggerrietts Geoff Gerrietts @ggerrietts • Turns out that’s not really true. • What separates expert from novice writers is not a superior understanding of rules and when to break them. • That’s not to say that the rules are not useful tools for learning • But that’s not how experts write: no expert writer is thinking about choosing a thesis and a topic sentence. • And they’re also not thinking about explicitly not doing that.
8. Revision, Not Writing • What separates expert from novice is skill at revision. • That’s not to say that the rules are not useful tools for learning • But that’s not how experts write • Experts write by successively revising their language for better effect.
9. Geoff Gerrietts @ggerrietts • For a novice writer, just committing ideas to paper is a struggle. • Revisiting for grammatical correctness may be the apex of the novice’s craft. • The rules will help here by giving them clear goals to reach for • and clear measures by which to judge their craft
10. Geoff Gerrietts @ggerrietts ...a day which will live in world history. • But for the expert, before words are on paper • Most of a sentence has already been composed in their head. • They have tested the flow of ideas from previous sentence to next. • Many word choices already tried. • Questions of grammatical correctness or paragraph construction never even rise to conscious decision
11. Geoff Gerrietts @ggerrietts ...a day which will live in world history. ...a date that will live in infamy. • Subsequent revisions yield more dramatic and effective edits • Clauses are promoted to sentences • Transitions adjusted for impact and rhythm • Words and phrases are replaced to better capture nuance
12. Geoff Gerrietts @ggerrietts Revise Proofread Writing Rewrite Prewriting The Writing Process • Incidentally, that’s why so many low-level composition classes feature an outline / rough draft / final draft approach • The structure of composition pedagogy is to encourage the novice to continually revise their work • At higher levels, composition classes are workshops, where students and professors all suggest revisions
13. Geoff Gerrietts @ggerrietts • So now you’re thinking “great; if I want to write technical documentation, I know how to get better.” • “But I don’t want to.” • And I understand that. • I’ve done that for a living, and now I have to force myself to even write good comments.
14. Refactoring, Not Coding • But just like learning to write is really learning how to revise effectively, learning to code is really learning to refactor • When we code, we are constantly revising the code we write • We jump back one or two lines to set a variable equal to 0 • We indent several lines to create a loop, or set up an if/then • The process of coding is actually the process of revising our code until it does what we want. • As we get better at revising our code, we develop the sense of “code smell” that steers our later instincts • So given this sentiment, maybe you’re starting to glean the broad outlines of my talk • But bear with me, I’ve got some more jokes in here, and some valuable insights, too
15. Geoff Gerrietts @ggerrietts The Grand Hierarchy of Coder Nerds • Let’s talk for a minute about what it means to progress as a programmer. • Well, it’ll probably take more than a minute. • But let’s talk about how we learn to program.
16. Geoff Gerrietts @ggerrietts The Apprentice • Maps logic to syntax • Unpacks complex logic by trial and error • Learns idioms and language features • At the beginning of the coding journey, we are all Apprentices. • Our primary exercise is simply mapping logical flows onto language syntax. • Translating “get a list of names” into “loop over User objects, joining first_name and last_name and appending the result to a list” • Or later doing the whole thing in a list comprehension • Often, complex interactions require successive rounds of trial-and-error revision.
17. Geoff Gerrietts @ggerrietts while True: # ... if done: break • Milestones might be learning idioms and language features. • Idioms might include things like list comprehensions, which are not obvious at first but are very handy • Or Python’s non-obvious way of spelling the do … while loop. • Or the “else” clause at the end of a loop. • Or even just the try / except / finally patterns for error handling
18. Geoff Gerrietts @ggerrietts • Validate Form • Raise Errors • Change DB • Render Template • Return Response • Idioms can also be domain specific • Like in this example of the steps involved in handling a web request. • These patterns are the very basic building blocks of programs, • Apprentices are getting their collection started, filling up their bag of tricks
19. Geoff Gerrietts @ggerrietts The Initiate • Has idiomatic fluency • Selects abstractions • Factors out units • Acquires good practices • The next level up is the Initiate. • An Apprentice becomes an Initiate when their focus changes • The Initiate has gained some fluency in idioms and language constructs • The Initiate may still be acquiring greater fluency • But the programmer is working on writing good code, not just correct code • Selecting good abstractions is the core of this • This makes the Initiate the target audience for a whole host of software rules
20. Geoff Gerrietts @ggerrietts • The canonical example is DRY which stands for Don’t Repeat Yourself • It’s broadly recommended, probably everyone here has heard it • This rule says you should create a function or loop where you see similar code
21. Geoff Gerrietts @ggerrietts A Unit is: UNDER 100 LINES (OR 10) Or perhaps: ONE SENTENCE, NO CONJUNCTIONS • Some of the rules I have heard are focused around the size of a “code unit” which is that thing we should be testing with “unit tests” • One guideline I’ve heard is that a unit should be no more than 100 lines. • I’ve also heard 10. • Programmers do love orders of magnitude. • Another guideline suggests that a unit should have a single purpose which you can describe in one sentence with no conjunctions.
22. Geoff Gerrietts @ggerrietts The Rule of Three • The rule of three is a variation or extension of DRY. • It suggests that you create an abstraction not the first time you do a thing • And not when you do a second thing that’s kind of similar, even though it might seem like you’re repeating yourself • But the third time you do something similar, it’s pretty evident that you know the correct abstraction.
23. Geoff Gerrietts @ggerrietts • Like our rules of composition, excellent code almost never conforms completely to these rules • But like our rules of composition, these guidelines are designed to help develop a programmer’s skill with abstraction • These rules offer guidelines for when and how to bump the level of abstraction without falling into overengineering • “Learning these rules” won’t make you a better programmer, and you probably won’t “decide to break them” when you’re good enough • But when selecting good abstractions is not automatic, using these rules to steer your decisions will help you understand what good abstractions are • And eventually, you’ll automatically select good abstractions with no reference to the rules at all
24. Geoff Gerrietts @ggerrietts The Master • Designs interfaces and architectures • Organizes abstractions into patterns • Debates methodologies • That’s when we start to think of you as a Master. • Masters have generally reached some conclusions about how and when to generate abstractions. • Concern has risen to the level of subsystems and systems, of architectures and interfaces. • Masters do not always agree, though.
25. Geoff Gerrietts @ggerrietts • FP vs. OOP • Static Typing vs. Testing • Compiled vs. Interpreted • 2 vs. 3 • Emacs vs. Vi • In particular, Masters often pick sides in the Holy Wars that perforate our industry • As Masters increase their mastery, discussion is generally less about which is better, more about when each is appropriate. • Of course for some of these, there really is a right answer. • I will leave it as an exercise to the reader to identify which is which.
26. Geoff Gerrietts @ggerrietts Patterns & Interfaces • Just as the Apprentice gained some skill mapping concepts to language, • Then the Initiate learns to abstract those language constructs into reusable units • So the Master learns to organize the units into cohesive patterns and interfaces • My use of patterns, here, is an explicit nod to the immense body of OO literature on the topic • But also a more general meaning, where small-p patterns emerge in software, and the Master learns to arrange their constituent parts • I personally place more emphasis on interfaces, but the two go hand-in-hand
27. Geoff Gerrietts @ggerrietts • Separation of Concerns • Single Responsibility • Principle of Least Knowledge • SOLID • GRASP • Plenty of rules and heuristics exist for masters • I’ve listed a few here: • Separation of Concerns suggests that you divide your application into separate, nonoverlapping components • Single Responsibility echoes our previous statement about the design of units requiring focus • Principle of Least Knowledge suggests that components should not know about other components’ internals • SOLID and GRASP are systems of principles for designing OO systems • Design patterns themselves are an orthodoxy • As before, these rules are useful tools for learning to design large systems • And for evaluating the design of large systems • And for talking about the design of large systems
28. Geoff Gerrietts @ggerrietts The Guru • Less a new phase • Apex Masters • Often named figures • But at some point, the application of the rules ceases to be conscious thought, and you have become The Guru • Sometimes when we say “guru” we really just mean smartest guy in the room. • But this kind of guru is like, if the room had a whole community in it • Generally see technologies in terms of trade-offs rather than silver bullets. • Can contribute to conversation at technology’s bleeding edge.
29. Geoff Gerrietts @ggerrietts Climbing to Code Nirvana • So that is the journey, from padawan learner to Yoda. • We start learning to map concepts to code. • We learn how to abstract those code fragments into reusable units. • We learn to arrange those units into systems and subsystems with tidy interfaces • As we develop, the smaller abstractions become second nature, and we learn to work with larger abstractions
30. Geoff Gerrietts @ggerrietts Coding is revision: • Ideas are expressed. • Similarities are found. • Abstractions are drawn. • In that way, programmers develop like programs do. • We factor repeated lines into functions • We wrap blocks in loops or conditionals • Later, we may recognize similarities between this code and some other code, and we may factor out a common abstraction. • Still later, we may group those common abstractions behind • This is the behavior of coding, just as it is the behavior of writing: constant small revisions with periodic larger revisions.
31. Geoff Gerrietts @ggerrietts Feedback Develops Skills • And advancement along our coder’s journey depends on doing just that • We continually revise our logic to fit idioms, until it becomes second nature to simply use the idiom • We continually revise our idioms into abstractions, until it becomes second nature to generate good abstractions • We continually revise our abstractions into patterns, until it becomes second nature to design good systems • Learning the rules at each stage of our journey helps shape our revision • But learning to code is learning to revise • And learning requires doing the revision until our brains are reprogrammed to “naturally” produce good code • Learning the rules has value, but limited value in practical application • What helps most is reading code, writing code, and receiving feedback on your code
32. Geoff Gerrietts @ggerrietts Code Review is the BEST. Really. • That brings us to the idea of code review. • Which I like. • Over the years, I’ve repeatedly participated in and advocated for code review • Some software practices require a little bit of religion: we know it’s a good practice because we know it’s a good practice • As software practices go, code review requires very little faith • The quantitative impact of code review on defect rate is well-documented • It’s also self-evident that code review reduces your organization’s bus factor • But the aspect I like best is how it hastens professional development. • All three of the things we need to get better are right there: reading code, receiving feedback, and revising.
33. Code Review as Mentorship • But to make code review a mentorship activity, we need to be conscious of how mentorship is different from hunting bugs
34. Geoff Gerrietts @ggerrietts EXTERMINATE • When hunting bugs, the goal is bug genocide • When code review is about the bug hunt, you don’t want to leave anything behind • And more importantly, all you’re looking for here is bugs: places where the program will not function correctly
35. Geoff Gerrietts @ggerrietts Difference #1: Improvements, not bugs. • But when we review as a form of mentorship, we aren't just hunting bugs • We are looking for opportunities to better organize the expressed logic
36. Geoff Gerrietts @ggerrietts Bugs break logic. Improvements just make things better. • The distinction can be fine but it's important. • A bug is actually a defect that prevents correct functioning. • Opportunities for improvement are mostly focused on the hygiene of the code base. • Importantly, bugs are demonstrably wrong. • Improvements are things that subjectively would make the code better. • We do want to identify bugs. • But we also want to identify potential improvements, and we want to treat potential improvements differently.
37. Geoff Gerrietts @ggerrietts Difference #2: Appropriate, not exhaustive. • High school composition teachers spend a lot of time correcting grammar and helping with sentence and paragraph structure. • Collegiate corrections focus more on the flow of argument, and how linguistic choices emphasize and shade meaning. • An instructor — or, later, peer — is not expected to list all possible improvements that they can think of, just the ones that seem most important. • A high schooler working on paragraph construction simply won’t be able to deal with feedback on how abrupt transitions can create emphasis.
38. Geoff Gerrietts @ggerrietts • With code reviews, though, it often seems to go a different direction. • Reviewers want the code being committed to be as close to “perfect” as can be • A huge laundry list emerges, some real bugs, some suggested refactorings • Learning and engagement with the revision process goes out the window • The reviewee adopts a besieged mindset. • “Let me just do exactly what I’m told so I can get this merged” • So don’t do that. Don’t smother your programmers in issues.
39. Geoff Gerrietts @ggerrietts Instead: Be relevant. • Instead, being relevant to the programmer’s development. • Meet the coder where they’re at. • Don’t try to engage them in a discussion of architectures and design patterns if they’re learning idioms. • And don’t waste an expert’s time nitpicking over minutiae. • Address them at their level. • Make them think critically about their choices • If you must address items beneath their level, treat those items like the trivia they are • If you’re going to address items above their level, you are leading them • If you’re leading them, provide leadership rather than provoking discussion • As an aside, “where they’re at” is usually something you can asses by “what do they need the most help with.”
40. Geoff Gerrietts @ggerrietts Instead: Be sparing. • Relevance is important, but so is focus. • Choose the important things to discuss. • Importance can be decided based on many factors: • An improvement might be important to the hygiene of the codebase • An improvement might be important to the programmer’s development • Or it might just offer an opportunity for interesting discussion • In composition workshops, “ten highlights, ten suggestions” is a typical rule of thumb • That’s awkward for code since diffs vary in scope • Maybe “Response to code review should not take longer than the code review?”
41. Geoff Gerrietts @ggerrietts Instead: Be even-handed. • That rule about 10 strengths and 10 things to improve is not just about scope, though • People respond better to positive reinforcement than negative reinforcement. • So try to identify strengths as well as weaknesses. • If you try to match them 1:1, you’ll get better at spotting strengths pretty fast.
42. Geoff Gerrietts @ggerrietts Instead: Be constructive. • And while you are looking for improvements, it’s important to be constructive. • This is important for everything, not just code review. • People respond very poorly to criticism. • Criticism is when you tell people what they’re doing wrong. • Criticism triggers defensiveness, even in people who “take criticism well.” Lots of experimental evidence for this. • People respond pretty well when you offer constructive suggestions. • Constructive suggestions are when you tell people what they can do better. • Be constructive. “This is wrong” will just make people mad. “Have you considered this” will open up a discussion. • Being curious helps here. • It also helps to remember that this is your peer. Assume diligence and good intentions.
43. Geoff Gerrietts @ggerrietts • Like everything else we’re talking about, giving code reviews that help other programmers grow is something you get better at with practice. • When done well, code review becomes a key mentorship activity. • It facilitates a round of guided revision that helps steer the programmer’s development.
44. Technical Debt • So we’ve talked about how the process of coding is a process of revision. • And we’ve talked about how repeated efforts at revision development coding skill • And we’ve talked about how code review can be used to help guide the revision process, to accelerate development
45. Geoff Gerrietts @ggerrietts The Elephant in the Room • Now let’s talk about revising the codebase itself. • Glibly, we can define technical debt as the sum total of all the things you wish you’d done differently. • More directly, technical debt is everything about your codebase that makes it difficult to extend and modify. • But let’s pursue that first point a little further. • Technical debt generally accrues when you either accept a certain amount of poor design • Or it accrues when you make poor choices when selecting idioms or abstractions or patterns or what have you • Or it accrues when you mispredicted the future and chose the wrong side of a trade off.
46. Geoff Gerrietts @ggerrietts Death, Taxes… and Technical Debt • It’s inevitable. • Technical debt is inevitable from two different directions. • Firstly, nobody has perfect forward knowledge of what the codebase will need to become. • You can’t know how to make every tradeoff, and sometimes you will need to make a call. • Second: everybody is still getting better at coding. • Not even Gurus write perfect code, or know for sure what ideal code is • In fact, what is ideal today will inevitably be legacy some day
47. Geoff Gerrietts @ggerrietts Sometimes, we even choose to make bad choices. • That’s before you even consider the emergencies. • Product needs a feature to launch fast, and we grit our teeth and do it. • Some project needs to just die, and so we push it out with known flaws. • These things happen to everyone, and there’s no shame in it.
48. Geoff Gerrietts @ggerrietts You will have tech debt. Plan to fix it. • No matter how hard you resist, you will have technical debt. • You need a plan to deal with it. • There are a number of fronts on which you can fight this battle, and I recommend committing to all of them.
49. Geoff Gerrietts @ggerrietts • The most common strategy is to limit the amount of technical debt you create. • To do this, you delegate design and coding tasks according to level of expertise. • The Gurus and Masters are tasked with architecture. • Initiates are given subsystems, often leading some Apprentices in building them out. • The Apprentices, in turn, are provided discrete and well-defined problems to explore. • Every organization I’ve been a part of does this. • Its effectiveness depends a lot on how your organization is staffed. • It also has the potential, if pursued too aggressively, to limit the growth of your people. • It’s still a good idea. No sense asking the intern to design your critical systems.
50. Geoff Gerrietts @ggerrietts • But prevention can only do so much. • Plan for remediation. • You need a cure. • Dealing with technical debt is essentially establishing a revision process for the whole codebase • Here again I see two main avenues of attack.
51. Geoff Gerrietts @ggerrietts Approach #1: Clean as You Go • Scoping technical debt into feature work is a very common strategy. • Doing so is taking a responsible approach to software. • The Boy Scouts have a principle: always leave the site cleaner than you found it. • It's irresponsible to do otherwise. • We clean our code as we go for exactly the same reason: it’s irresponsible to do otherwise.
52. Geoff Gerrietts @ggerrietts • However, this has some shortcomings
53. Geoff Gerrietts @ggerrietts Can Make Features Prohibitively Expensive • Sometimes, when you scope in the cleanup work you want to do • The product team decides the feature is too expensive • So now there's no feature and no cleanup! • Can be tempting to back down here, but consider creatively re-framing the problem • Can you do most important cleanup? • Can you clean it in phases? • Generally don't want to double the size of a feature with cleanup tasks. • Trying not to exceed 1.5x is probably a good goal. • Of course, sometimes there’s no way forward but through. • That’s when you find out how much your business team loves you.
54. Geoff Gerrietts @ggerrietts Only Fixes What's Up Next • The other problem with cleaning as you go is that you only fix what you’re working on. • Say you finish a big initiative. • Ideas are still fresh, and there's a list of things to improve. • But now the team is building something else. • It may be a year or more before you revisit. • All that knowledge goes stale! • Similarly, sometimes a particular chunk of code keeps kicking out bugs like some kind of reverse roach motel • You need to get in and refactor that code to get some of the smell out of it and correct the way it models the business problem • Sometimes, the code that is causing you the most trouble, isn’t the code you’ re directly working on.
55. Geoff Gerrietts @ggerrietts Build in Maintenance Time • This is where the second avenue of remediation comes in. • We build in maintenance time. • Now, like some sort of perverse binary tree interview problem, there are a couple ways to do this, too • I think of them as the ideal way, and the pragmatic way.
56. Geoff Gerrietts @ggerrietts Approach #1: Prioritize Technical Debt Against Feature Work • The ideal way prioritizes each bug and improvement against upcoming feature work. • Priorities are negotiated among all stakeholders, and a single unified backlog results. • This is hard to do: different stakeholders for each, and each has a different value proposition. • It’s cider fans and OJ drinkers comparing apples and oranges, and it’s hard work understanding the other’s perspective • Technical debt frequently has a long-range ROI that is hard to quantify • The payoff is typically a reduction in the complexity of the codebase, which in turn affects velocity • Product representatives have a tough time measuring that against the ROI model used for features • Usually, they punt
57. Geoff Gerrietts @ggerrietts Approach #2: Fixed Maintenance Budget • The pragmatic approach just sets aside a certain amount of capacity for maintenance. • Most organizations should have no trouble supporting between 20% and 30% to cover maintenance. • Usually, maintenance will include resolution of production bugs as well as resolving technical debt. • Teams might like to keep those two line items separate. • I would recommend no less than 10% for technical debt. • Personally, I prefer having the larger bucket to split up according to current priority.
58. Geoff Gerrietts @ggerrietts • So, in a sort of curious as-above, so-below situation, we see that revision drives software. • Small cycles of revision power the developer’s coding process. • Code review leverages larger cycles of detailed feedback and revision to help develop programmers. • And the largest cycles of revision operate on the codebase itself, where a whole team learns to revise the collective project • At each level of this process of revision, practice and feedback increase the effectiveness of the individual, the team, and the effort • And, perhaps most importantly, insights into good revision practices obtained at any level can be repurposed in the other activities
59. Geoff Gerrietts @ggerrietts • Right. • So what are good revision practices?
60. Geoff Gerrietts @ggerrietts Actually revise. • This one is kind of key. • The more you look back at your code to change it in light of what you’ve just recently learned, the more you learn • The more you consider code you’re working on as an opportunity to clean up, the more you’ll clean. • Seriously, the place where most people fall down • The place where most revision habits suck • Is that people just don’t bother to revise
61. Geoff Gerrietts @ggerrietts Be self-critical. • I know I said some stuff back a few minutes ago about criticism. • And it’s true, you shouldn’t just criticise yourself then stop. • But it’s important to look back at what you’re doing with a critical eye. • What are the assumptions you made, and do they hold? • Was that abstraction really right? • Are you holding onto a cute or clever idea because you love it, and not because it makes things better? • Were you lazy about error checking? • When the answer is yes, move on to the constructive next step. • But police yourself, because your code reviewer is not your maid and having a tech debt process isn’t a free pass
62. Geoff Gerrietts @ggerrietts Actually revise. • Yeah I duplicated the slide. • I’m pretty sure you nodded along the first time but blew me off. • But seriously, this is the main thing you have to do. • The secret to becoming a better coder is aggressively revising your code. • The secret to successfully revising is to actually revise. • How simple is that?
63. Geoff Gerrietts @ggerrietts Thanks! Questions/comments: ggerrietts@appneta.com @ggerrietts