Agility and code quality

Some days ago, I attended a meeting, where different projects’ agile people met and talked about challenges, achievements, stuff. Actually it was not the first time we met. The meeting has been held for months now. For these long months, there has been something that was bothering me about these discussions. I couldn’t name that thing, so I stayed silent, but now I think I suddenly recognized it (under a hot shower, sorry if it’s TMI – too much info). After these months it seems to me that people percieve agile as daily stand up meetings + retrospectives (or just the first one, in case of Kanban development). Some of these people are amazed that they’re still getting bug reports in spite they “are doing agile”. Standups: check, retros: check, planning meeting: check, but bug reports, however, still flowing in.

Don’t get me wrong, I’m not against these things. Daily standup is a very important thing. It facilitates communication between team members, or sometimes even between different teams. Daily scrum alone, however, will not make code quality any better. It will strengthen team spirit, it will help everyone to keep track with every aspects of the current development, but it’s not meant to get source code in a better shape.

The same applies to retrospective. It is a very cool forum to talk about strenghts and weaknesses of a team. Even better if constructive ideas arise (by the way, I don’t understand why some people think retrospectives cannot fit into Lean/Kanban ways of working. The process itself can be trailored and developed; for new teams it can be particularly helpful). In my opinion retros are one of the most important meetings of agile frameworks. It’s psychology, I guess. Some people simply won’t tell their ideas, unless they are told to do so.

So, standups and retros (along with other meetings) fail to improve code quality. That’s because agile ways of working  can be divided into two parts:  the process itself and a technological part. The process part mainly contains of the meetings described above, definitions of done, team rules, team members’ motivation etc. One needs soft skills in order to manage/take care of these things. By soft skills I mean empathy, friendliness, motivational skills etc. And all these things will not get you a good quality product. It’s because quality is directly related to technical skills. Technical skills include test driven development, code review, pair programming, design discussions and so on. So, will code review help improve quality? Most probably. You might spot suspicious code lines, liar names, uncovered production code. Will TDD help? Definitely. Your code will be doing exactly what you expect it to be doing.

I once heard of a project where all the soft skills were perfectly applied. All staff members were happy with their work, they have had daily scrum, retros, everything. However, when it came to unit tests, they were like: “well, uhm, our unit tests require a living connection to the database, a writable file system, a network connection …” and so on (remember the definition what does not count as a unit test: a test which touches the file system, talks to a database or over a network. Anything that lives outside of the test runner framework). So even if these guys respected all the soft-skill related part of agile, were producing not so good quality. They were stuck at a low level of technical skills.

So what I’m saying; it makes no sense to apply only one or the other part of an agile dialect in solitude. You need both of them in order to succeed. Soft skills in order to get motivated and happy people, and technical skills in order to produce fewer bugs and maintainable architecture. No process hsould be called agile until both parts are applied, in my opinion.

Advertisements

The power of words

The power of names actually…
I’ve come across this code the other day:

private boolean resultOfComparison;
public void setResultOfComparison(final boolean resultOfComparison) {
this.resultOfComparison = resultOfComparison;
}
public boolean getResultOfComparison() {
return this.resultOfComparison;
}

So, what is wrong with that code? Nothing in particular, it compiles and runs fine, so it’s perfect. Well, it is actually, until other people start using it and making assumptions about what resultOfComparison really means. (seems a little bit awkward to me to name a boolean variable resultOf… It’s either true or false. A result -for me- is something that you cannot guess with a chance of 1:2). Basically resultOfComparison was meant to keep track of whether a repository is up to date or not.

Since we as people are different we re thinking differently; We also like to be lazy a bit (not to browse through the code but make assumptions). It just happened in this case too. As different people had worked on the same feature, they imagined differently what resultOfComparison should have been in different situations. The following two pieces of code have been created:

//Class1
if (resultOfComparison) {
//do something
}
//Class2
if (!resultOfComparison) {
//do exactly the same thing
}

The first guy thought resultOfComparison must be false when the repository is up to date. The other one was sure that it has to be the other way around.

Yeah, not a big deal, one might say. Just correct one of the classes according to business logic and the glitch is solved. True. And easy to see indeed. But what if you have a system of 3000+ source files? Then you end up with a dysfunctional system, have to cancel a demo and spend a whole day in debugging.

The whole thing could have been avoided only by giving the variable a meaningful name, like repositoryUpToDate; so everyone could have seen how the particular thing have to behave runtime. The worst thing about this annoying bug was that all unit tests were in place. And all of them were green. They were based on the two different assumptions too…

I’ve refactored that name a bit, it became isInventoryUpToDate, so with a quick “find references” I was able to track all the issues down. Then all the broken conditions were trivial to fix. Kind of annoying bug; a whole day wasted. 😦