How should I implement code review in this situation?

Question

My question is about how to put code review into practice in a workplace where nobody else has asked for it, I've never done it before, and I'm not entirely sure if it's worthwhile for this type of programming.

I'm a contractor on an assignment that mostly involves tools engineering, data analysis, fixing code when someone's Excel template changes break a macro, etc. It's total cowboy coding: Requests are more "do what you need to do to hack together something that works ASAP, we need it in production tomorrow morning and we're only going to need it for a couple months" than "make a well-structured application that will be easy to maintain and develop under all foreseeable circumstances."

The software engineering team is me and one other guy whom I supervise. We don't currently follow any kind of software development methodology beyond "make something that fulfills the functional requirements", and neither of us have ever worked anywhere that had any code review or design principles. Everything we make is either VBA or C# desktop apps, and each of us is usually working on a separate assignment rather than collaboratively building the same program. There is no version control system in place and we don't have server access to install any such system.

What I'm trying to figure out is, given that the work is very ad hoc, what kind of code review, if any, should I put in place? Or is it even necessary for the kind of work we're doing?

Answer 1

Are you doing your own QA as well? Your situation sounds a lot like the last 3 I've been in, and I suspect it's a lot more common than all the documentation on best practices (code reviews, documentation, SCM etc) would seem to imply.

My input is that at least you have the other guy! Better than it being all on you. I'd suggest each of you doing the QA on the other guys code and giving it a quick read to make sure each of you can understand what the other guy has written. This will get you better code because the other guy doing the QA may not make the same assumptions as the author and might catch more bugs than the author would, and since you each may end up working on the other guy's stuff, you want to ensure at least a minimal level of readability / maintainability. I've inherited stuff that was obtuse and barely documented and the only way I could figure it out was to painfully step through the debugger. Readable / maintainable code is not a luxury, it's an investment.

I agree with the other guys too - pair programming might be a good fit, and definitely get at least an SCM.

Answer 2

Well first thing is to get a SCM of some sort, even if its just git shared between yourselves without any form of central/golden server repo.

Then I'd say install Redmine and use that to track the change requests you get - because you can point redmine at your git repo and see the changes that were committed, but for this to work well you'll need a golden repo and a server to install redmine on! Of course, you can get away with using your own PCs for this task, but one of you will have to be the guy hosting this - and therefore be the guy who 'owns' the definitive repo. I assume this guy would be you as you're interested in it, and I'd really recommend this approach. If you're scared of using git, or don't quite trust yourselves to manage it well (as it can be confusing and complex at first) then install VisualSVN Server and get the other guy to commit to that instead - setting up SVN is easier both from tools PoV and conceptually. You can always change to git later if you realise the benefits and can force the required workflows on the other guy.

This will let you start with the basics of SCM tooling, and maybe in the future you can get a server and do something more organised and 'official'.

Oh yes.. code reviews... until you have more staff, do manual code reviews - look at the changelog in Redmine, double click it and it'll show you a pretty graphical diff of the changes for you to review. No need for a more formal tool that will send you emails, bug tickets or whatever with a link to the checkin diff in this case.

Answer 3

Requests are more "do what you need to do to hack together something that works ASAP, we need it in production tomorrow morning and we're only going to need it for a couple months

Based on this, the focus of your code reviews is probably not maintainable, stable code. However, you may still profit from code reviews as a means to further develop your and your colleague's programming abilities and to develop a common understanding of the problem. As @Mike suggests, pair programming on specific problems might turn out to be a very effective way of employing some sort of code review in your scenario. Pair programming will lead you to discuss the approaches to solve a problem and therefore may help you to "hack together something" with less bugs even faster, which is something that directly translates to business value.

Answer 4

I agree with the pair programming suggestion, however if you can't afford it, I find pre-commit one-on-one code reviews to be a good compromise collaboration wise. If you don't have a source control system and thus don't do commits, well first of all you should absolutely get one (a DVCS in your case ?) :) and in the mean time you can do these code reviews just before release, i.e. before publishing the application installer on a server, handing over the Excel template with the updated macro to the user, etc.

Each time a significant, complex enough update to a program has been written and needs to go live, the author should from his own initiative ask for a complete collaborative review of that code. This takes time but it takes that to spread the knowledge across all team members, establish collective code ownership and perform a first pass of internal quality control on what's produced.