There Is More To Clean Code Than Clean Code

A post written by Uncle Bob in January (I'm behind my reading list) offended me. I absolutely agree with Uncle Bob's analysis regarding the code itself, and I also prefer the refactored version, but I have a problem with insulting the programmer(s) reluctant to appreciate the change.

We write code in programming languages, and there are different levels of proficiency in a language.

As I'm currently learning a new spoken language, I'm painfully aware of this - initially I probably sounded like a caveman. The first impression you get about me is totally different depending on the language I speak - but I am the same person!

The learning curve of a language is not smooth - the steepness between consecutive levels of proficiency is different. Going from not speaking any German to speaking A1 (tourist) level was easy, getting the basic grammar required for the low intermediate (B1) level wasn't too bad, but to get my German to the level where my English is will take more effort than the sum of all my previous investments¹.

Since it is my third foreign language I'm learning, I have no difficulty accepting that the level I think I speak is higher than the level I actually speak. Because of that, whenever someone rephrases my sentences in proper German ², I start from the assumption that likely their version is better, even if I don't understand first why - and I take the effort to understand their reasoning ³. I do that despite that I was of course convinced that when I spoke, I expressed my thoughts in the best possible way.

However, I don't have much at stake - no ego to hurt, no reputation to loose, and the roles are clear: I'm the beginner, and the people around me are the more experienced ones. In a software team, the roles might not be so clear - I had told colleagues almost twice my age how they should write code after only a few weeks of working there. Bad idea. Since then, I have learned not to start improving the coding style of a team by rewriting the code they have written, and showing off how much better my version is. Rather, I wait until a situation arises when they don't mind having me demonstrate some code improvements. I demo it, and explain why I do it that way. In my experience, the second approach is more effective, though it doesn't have that instant satisfaction and relief the first provides.

As the joke goes, you need only one psychologist to change a light bulb, but the light bulb has to want the change real bad.

Driving Technical Change is hard, because it requires a mental/cultural change, and that change has to come from the inside - but can be catalyzed from the outside of course⁴. But just forcing practices or ways of working on unwilling recipients generates resistance (e.g.: the story of the EU technocrat appointed to recalculate the 2009 Greek budget).

I would like to see more public code reviews and public refactorings (e.g.: Andrew Parker, GeePawHill), but I would like to see less public judgement passing on people at the lower proficiency levels of programming.

---

¹ there is a great Hanselminutes episode on learning a foreign language if interested. Beware, it may contain programming!

² German readers might disagree, since most Germans I meet speak Frankish :)

³ Which of course, is sometimes harder for natives to properly explain than for novices to ask questions pointing out the seeming irregularities of the grammar

⁴ And we won't always be able to foster change in all environments (note: this does not mean the others are at fault for not changing!). The same programmer can be highly productive in one team, and be the one slowing down another team. There is nothing wrong with changing jobs after realizing we are a net loss to a given team.

On Grassroots/Peer TDD Introduction

While I'm no trainer/coach, I've been involved in spreading TDD among my peers, with varying extent of success. Recently someone asked for advice on how they should go about it, and in the spirit of Seth Godin, instead of responding in a private email, I wrote this post, a mix of experience and hindsight ideas.

So, we are one (or two) developers in the team/company, already sold on TDD, with some unit testing experience. The peers are willing to listen to the concept, and management, while not willing to invest in formal training, is ok giving a chance, as long as it doesn't hurt the projects' performance. How do we go about it?

Don't introduce TDD on the project

It's common that teams learn/practice new skills on live projects after a basic overview/training. While this might work, I would not recommend this approach for any grassroots initiative, because the chances of failure are significant, and one bad experience with a practice/technology can doom any further attempts to introduce it later, even if not the practice was at fault, only was applied without enough practice. A project manager friend of mine recently complained to me that since the team that worked on a massive legacy project started doing TDD, tasks that used to take hours begun to take days, and clearly, he wasn't happy about that. It turned out that the problem wasn't due to TDD, but lack of discipline (developers went overboard with refactoring, touching parts of the codebase not even remotely connected to the task they've been working on). Despite this, he could have easily come to the conclusion that TDD equals to dropped productivity, without much benefit (bug reports kept coming in, even if for different features - we are talking about legacy code!). Any new skill has a learning curve, you will be slowed down, and you won't get it perfect the first time - don't give a chance for the others to associate failures with the new concept. If you are like me and would need a more concrete example about refactoring just enough, I suggest you read Ron Jeffries' Extreme Programming Adventures in C# book, and on advice for dealing with legacy code, the undisputed classic book is Michael Feathers' Working Effectively with Legacy Code.

Introducing/selling TDD to the other developers

A lot of technical presentations focus on the how instead of why it is beneficial to the audience. When it comes to TDD, I would also suggest presenting it simple - skip the emerging design concept (some of the early advocates noted that good design emerging from TDD might had to do with the fact that those early advocates actually had great design sense anyway), don't mention YAGNI - in short, don't overload them with new concepts. I rather liked the approach Paul Butcher took in his Debug It! book - we tend to program one baby step at a time, articulating in our mind what the next behavior we'll implement should be, then we code it, and move to the next behavior. TDD (or Test First Development for the nitpickers :)) is just a minor step from that, i.e.: we actually write down the articulated concept in code (which we tend to do anyways with the throwaway main methods used for debugging). And in addition, we get an automated regression test suite for free :). While a live demo could be cool, I would probably show one of the Kata casts instead of me typing on the projector. Also, be sure to point out that at first we won't be able to code at this speed (I still can't)!

Learning/practice

TDD is a rather simple concept, but so are all the great wisdoms - it takes a lot of time and experience to fully understand those concepts. After the initial TDD sales pitch, I would dedicate a session to going over the tools and the basic usage - how to install the test library, how to run the tests, how to debug a single test, and the basic assert statements - AreEqual vs. AreSame, Assert.Throws, and also an example to show that most likely there are more asserts in the library (e.g. StringAssert.EndsWith).

With those interested/committed, we can move on to the supervised practice step. We've worked on simple problems, in a structure inspired by this Peer learning presentation by Jason Gorman (his site even has a full case study of this applied at the BBC). The coding dojo is another possible format. Important thing is discuss the experiences, lessons learnt, and problems overcome with the whole group, because while practice makes one perfect, bad, practiced habits are hard to unlearn. It is OK not to have all the answers - just be ready to admit it, and ensure that you can find it out from somewhere. Meetups, user groups, and similar forums are invaluably helpful.

Be sure to eat elephant in a piecemeal fashion - while at first the simple problems seem contrived, don't attempt to practice testing existing, complex production code. Stick to the simple problems for practice, every now and then actually demonstrate that it is possibly to test legacy code (though it'll require quite some preparation from you), maybe have some of these examples as a group exercise, but be sure to keep the practice focused on the simple mechanics initially. Just like any sport practice - you repeat rather basic exercises over and over, because it does pay off in the long run, but to keep you motivated, you do have the free practice sessions too.

Don't force it

Not everyone has the same passion as you do (you must have if you read this far :)), and some take a longer time to learn, have different pre-existing concerns they should overcome. Lead by example, be available to talk about it, answer questions, clarify, but don't try to force it on them. Urban cycling didn't became popular because we, cyclists, were preaching to pedestrians and car drivers - we were just approachable to friends, coworkers, etc. and provided answers and listened to their concerns. Yes, it could take a long time, but patience pays off - self-motivated people are more likely to overcome the hurdles they inevitably will face in the learning process!

Bringing it to production

I wouldn't set strong rules or hard deadlines like "two months from now, you all must write all production code in a TDD fashion", but rather just tell people to feel free to experiment during their daily work, but ensure they don't confuse work and practice. If they get stuck on testing some problem, they should just make a note, and deliver the feature untested, as they used to do before. Later, together with the group they should discuss it and potentially find a way to test it.

Further resources

This post is just a high level overview, and I have simply glossed over many of the actual difficulties about writing good unit tests, keeping them maintainable, etc.. There are books written about the topic, in almost all programming languages. Coming from a .NET background, I can recommend The Art of Unit Testing with examples in .NET by Roy Osherov, and I've found James Shore's Let's Play TDD Java episodes great - the best thing about the latter is the thought process narrative that goes with it. Driving Technical Change might be another good book, though I haven't yet read that.

Good luck!

How & Why I do code reviews

Code review is one of the programming practices that I advocate. However, as with any practice, I find understanding and communicating the values and driving forces behind it even more important than the actual practice itself (see: Cargo Cult). So, here is my take on code reviews.

Why?

I'm pretty much against introducing any practice just for the sake of introducing it. Different goals require different means, and without clear goals, it's rather difficult to evaluate whether a practice has helped the team or not. The reason I practice code reviews are:

Knowledge sharing (note: none of the teams I've been part of practiced pair programming, which serves this purpose quite well). It's a way to break the silos that exist within the team, regardless if they are horizontal (UI, server side, data access, etc.) or vertical (this is Jane Doe's component, noone else can touch that for there dragons live). I rather like that when I ask for a holiday my manager doesn't have to consider whether or not there is anyone who could work with my (recent) code.

Mentoring, though could be considered part of knowledge sharing, deserves to stand out on its own. I've found a lot of the concepts I came across first way too abstract, when read in a book, but when explained in context (what you've just written there is known as the Factory pattern...) I get a better and deeper understanding of it.

Culture creation. Again, abstract vs. concrete, talking vs. doing. One can have so many coding guidelines, best practices, etc. documents written. However, these documents usually highlight high level objectives, and almost any concrete implementation can be argued to fit with the description. I much prefer people arguing whether or not a new class should be extracted or not to a dry design guideline setting limits on the number of methods of a class and just checking against that formally (if at all).

Quality Assurance. Are there enough and appropriate tests? Do they makes sense to the next person (I routinely had to rename tests for my reviewers, because I was already too close to the code and didn't communicate intent well enough)? Are actually all tests passing? Haven't you accidentally reinvented the wheel (I've once failed to recognize that what I'm working with is an existing domain concept - the beauty of working with legacy code - so I created another model class for it)?

Last, but not least, catching obvious bugs is a benefit not to be overlooked. Face it: we all make mistakes, that are embarassing in hindsight. Catching them during the development cycle is keeps the embarassment in a smaller circle :)

What?

Computers have been invented to automate tiresome, repetitive, and boring tasks. Spending programmer time on checking tabs vs. spaces, curly brace placement, and other such, easily automated checks (StyleCop, Checkstyle, FindBugs, etc.) is more than waste. If such things are important for you, do a quick check that there are no warnings in the build for these, and move on to the areas where value is added.

When?

Code reviews should be concerned with small, easily digestible chunks, just like commits. That's why I prefer to review commits. Before they are commited. I can only offer nonscientific justification (experience + anecdotal), and no explanation, but my observation is that committing code makes it just as rigid as clay becomes in the oven - it becomes much more difficult to change it. Renaming a method/variable is much easier while it's still in the staging area (maybe the burden of writing another commit message?). This also gives enough time to ensure that tests are run before each commit (run the command line build while the review is ongoing).

Some suggest that developers should ask for code review when they feel it's a riskier change they've made. Since I'm really bad at predicting when I make mistakes (to be frank, I tend not to be able to predict), just to be on the safe side (and to ensure the flow of information), I advocate to review every commit.

How?

There are multiple ways to achieve the same thing, one that I've found working is the following:

1. the developer who just completed a commitable chunk of work, signals the others (via a team chat window, though ACM ICPC style balloons could work too) that she can be reviewed.
   1. someone from the team picks up the request in the next few minutes - this delay ensures that the reviewer is not interrupted and dragged out from the flow.
   2. If noone picks it up in a few minutes, the developer stashes his work, and continues working until someone becomes available, when they return to the original change set.
2. the command line build is started on the developer's machine while the nature of changes is described in a few sentences to the reviewer, so the context is set
3. the reviewer drives the order the changes are reviewed. She asks questions, requests clarifications, which usually result in rename refactorings.
   I tend to start reviewing the tests - first reviewing the names make sense and no relevant corner cases are missed, then the actual test code. Only once I'm happy with that do I turn to the actual code.
   1. if any major flaws (bug, failing test, duplication - anything that cannot be fixed as fast as discussing it) are discovered, the code cannot be commited, and another review is needed (we tend to have that next reviewer be the same person as the one before in order not to slow down the process).
   2. otherwise, the normal checkin dance begins, and the code is committed.

Cons, worries, and pitfalls (real and unreal ones)

Just like any other practice, this takes time to learn and to do. However, I rather prefer the extra few minutes spent discovering duplication than the days it'd take later to remove. Exposing the potential misunderstandings that wouldn't be discovered while just talking again justifies it. If a review takes longer than a few minutes, that signals there is a deeper issue there, which should be discussed - like agile practices promote, we've exposed a problem early. In summary: I think it's worth it.

Co-location, while certainly makes the process easier, is not required. If you are working in a remote team, you probably have appropriate communication channels. A voice call with shared screen is all that's needed.

It takes effort and discipline. If the reviewer doesn't pay enough attention, isn't engaged, it's not going to work. Discussing regularly what kind of problems slipped through the past code reviews and whether they could (should) have been caught could be part of retrospectives.

Just as it's possible to create horrible designs with TDD, it's just as easy to deviate from the architecture, design of the program in small, correct steps. Even when reviewing only small chunks, keep in mind the big picture.

Some teams do post commit code reviews instead, arguing that if we do it in an asynchronous fashion, the work of others is not interrupted, the developer is not slowed down by waiting for others. While I've seen teams practice post commit reviews effectively (as they say, a team of good developers is likely to achieve good results regardless of the process/practices they follow), so I'm not saying that can't work, but I prefer to do it before. In addition to the clay analogy used above, synchronous communication is more effective than asynchronous (at least between humans, between machines its another story). At the end of the day, developers will still need to talk with each other for clarification, so not much time is saved anyway.

Tooling. Most code review software don't support pre commit reviews, however, they have value for other purposes. I like them to start discussions on things I see in the code that don't need immediate changes for the features we are working on now, but should be discussed at some point - either to become a nice to have task, or to actually realize it's a must have refactoring. I find these tools can be more effective than leaving TODO comments in the source code - threaded discussions are nice, and hard to have in code comments. Another area where they shine is architecture/prototype reviews. It's a great way to prepare for a design review meeting. And yes, I do prefer code based architecture evaluation to UML diagrams (though the later are useful when communicating about the architecture), because concrete coded scenarios can bring up more flaws than the white board - see the Non Coding Architect.

And do I need to say it? Just as any other practice, it's no silver bullet!

---

With regards to silver bullets - nothing is perfect, including this approach and the post itself. Please, do leave a comment pointing out any improvements/problems!

---

Update: Code Review During Retrospective lays out a great approach for purely educational focused code reviews, I recommend reading that too!

Dealing with crunch mode

Before going any further: I think that crunch mode is not sustainable and is best avoided. I won't analyze the phenomen or its causes, there has been much written about it. However, given it definitely exists, though usually for shorter periods of time. Having had this experience a number of times, I felt I should organize my thoughts for future reference (for myself, and maybe for others too).

Some of the points below might sound like good practice for software development in general - often we only start working on process problems after they've blown up into our face. Holding retrospectives after such periods can bring quite a change, so don't miss out on the opportunity! However, we haven't even started yet, so let's step back in time.

Knowing why the close deadline is important from the beginning is crucial to keep the team motivated during this period. Working overtime for no apparent reason makes the already bad situation worse.

Have well defined boundaries for both time and scope. This is generally a good practice, but this is a must - setting out on a death march without a clear purpose is unlikely to succeed. Break things down into small (4-8 hours) tasks, prioritize them (and I mean an ordered list, not making all of them into critical priority). If the added up estimates already run out of the possible available time, cut scope. Most likely the only way it can be cut is vertical - e.g.: we will have logging, but we will work against only a single library, and won't make it pluggable for this deadline. Or we won't make the logging destination configurable (though it's a contrived example, it helps making the point).

Taking on technical debt can help. Be sure you all understand that this does not equal to crap code, unconditional copy-paste, etc.. Only introduce debt knowingly (you might want to keep an actual list of the shortcuts you have taken so you know what to fix in the upcoming releases).

Stop experimenting and move back to the comfort zone. Almost all projects involve some new element that the team is not familiar with it. If the team is just learning unit testing, they likely write the tests after they got the code to work, and are taking a long time to write them. In that situation, not writing tests for the time being could be the right choice. However, be careful to get the right message across - you are not dropping the practice because it slowsdevelopment down, but only rescheduling the learning period to some other time. Also, just like with performance optimizations, make these decisions after measuring - don't do itbased on speculation.

On the non-technical front, you can cut back on meetings, both in numbers and in duration. You might also want to schedule them so they don't interrupt the workflow. Have a status in the morning and at the end of the day, with a team lunch if more discussion is needed.

Work from home. Of course, some infrastructure is needed, and it might not be an option for everyone, especially for the person where next door is a construction site. However, saving on the commute time might could give you some extra time during the day without cutting too much into your personal life. Significant Others are more understanding if you are a bit more tired when they get to see you at least.

---

[Scope section was updated based on @ljszalai's comment on twitter]