I'm working on a project solo and have to maintain my own code. Usually code review is done not by the code author, so the reviewer can look at the code with the fresh eyes — however, I don't have such luxury. What practices can I employ to more effectively review my own code?
-
35I'm not sure you can, at least not effectively - you can crowd source a review team at codereview.stackexchange.com if your code is not proprietary though– jk.Commented Mar 12, 2012 at 11:03
-
8You cannot review your own code. If you cannot get other humans, I would consider the best you can do to use as many static analyzers you can get your hands on, and enable ALL warnings.– user1249Commented Mar 12, 2012 at 15:31
-
143Reviewing your own code is easy. Write a piece code. Step away for 2 weeks/months/years as you continue to learn and develop other software. Come back to that piece and try to understand what the code is doing. You know you learned something when you think: "what kind of idiot wrote this?!".– Yuriy ZubarevCommented Mar 12, 2012 at 18:03
-
7@YuriyZubarev But what if you don't want to wait for weeks/month/years?– anatoliiGCommented Mar 14, 2012 at 17:40
-
12You can review your code in an altered mind state. Or you can code in an altered mind state and delegate a code review to your normal boring self.– SK-logicCommented Apr 26, 2012 at 8:00
18 Answers
First of all, make use of tools to check as much as you can. Tests (backed up with some reasonable code coverage) will give you some confidence of the correctness of the code. Static analysis tools can catch a lot of best practice things. There will always be issues that you need human eyes on to determine though and you will never do as good a job reviewing your own stuff as someone else, there are some things you can do to help however
- check tests exist and pass (possibly have a target test coverage, though you may need to break this in certain cases, but you should be able to justify why)
- check Static analysis passes (there will also be false negatives here but that is fine as long as you can justify why then its fine to suppress them)
- maintain a check list of further things to check in review (ideally add this as new static analysis rules if possible) make sure you check anything the SA can't check, e.g., are comments still valid, are things named appropriately (naming things is of course, one of the 2 hard problems known to computer science)
- if a fault is identified check if the cause is systemic and look at why it wasn't found in earlier tests or reviews
This of course is useful when you are reviewing others code as well
-
3Concerning the check-list, having a spec is super useful. Commented Apr 25, 2012 at 16:36
-
1I don't like checklists. They make reviewer concentrate on checklist instead of thinking the problem, the solution, and many other things. So I suggest make them minimal. Commented Jun 6, 2013 at 3:50
-
@BalogPal I agree with your point but in the context of a self-review this is different. A checklist here prevents a developer from rubberstamping work when they already think they understand the problem and solution (from writing it in the first place), instead intentionally forcing them to disconnect from their "developer goggles" and forcing them to look at it through reviewer goggles instead; to make sure they aren't blindly accepting their own work.– FlaterCommented Dec 9 at 5:58
Take a look into the Code Review Stack Exchange site. It is for sharing code from projects you are working on for peer review:
Code Review Stack Exchange is a question and answer site for seeking peer review of your code. We're working together to improve the skills of programmers worldwide by taking working code and making it better.
If you are looking for feedback on a specific working piece of code from your project in the following areas…
- Best practices and design pattern usage
- Security issues
- Performance
- Correctness in unanticipated cases
You can also use code static analysis tools to detect certain kind of problems, but they are going to produce false alarms in some cases, and can not suggest how to improve the design.
-
3That's an excellent answer to the question "How to get my code reviewed", and a good advice in general (I will definitely do that) — but still a little offtopic. Commented Mar 12, 2012 at 12:04
-
5I normally don't like a 5 word answer, but this one is just so right.– maple_shaft ♦Commented Mar 12, 2012 at 12:37
-
21At best this is only a limited solution. Continually putting your entire daily output on CR.SE isn't feasible since large chinks of it's going to be fairly mundane boilerplate code. CR.SE is also not going to be much help with spotting problems that require a non-trivial understanding of the entire application architecture or domain the app is written for. From informal, look at coworkers code when it's checked in style, reviews where I work these sort of errors are probably more common than the ones that would be suitable for catching via CR.SE. Commented Mar 12, 2012 at 13:12
-
3The real value in reviewing is getting pieces of code you never though would ever present any problem spotted and highlighted as non-obvious nor self explaining or even non logically correct. You can't post the snippet to
code review
if you are not already aware it's a problematic one.– ZJRCommented Mar 12, 2012 at 13:31 -
3@ZJR Well, is the code in your projects 100% reviewed? If yes, your engineers have too much free time. As for your 2nd comment, I don't see problems in asking for a code review in a code that you think is perfect.– BЈовићCommented Mar 12, 2012 at 13:40
I agree with jk-s opinion that single-person review is not as efficient as 2 person review. However, you can try to make the best of it:
short term review (shortly after the code was produced)
I am using git as a local repository. Whenever I have finished a feature or fixed a bug I transfer the changes to the repository.
Before I check in, I compare what I have changed in my code and rethink:
- Do variable/method/classnames still reflect what they are used for?
long term review (6 months after the code was produced)
I ask myself:
- Can I describe in one sentence what a class/method/variable does?
- How easy is it to use a class in isolation (without the other classes) or to write a unit test for it?
-
4+1 for short term review suggestion. Using git to view all changes between different points in time can really help cleaning up the code.– LeoCommented Mar 12, 2012 at 14:31
-
I quiet like the long term review idea, I think I'd probably combine it into a general project wash-up review though and maybe not review all code (then again I don't tend to do solo development much)– jk.Commented Mar 12, 2012 at 16:13
-
I try to do something in between: review my code in about a month. I also like the 6 month review.– David GCommented Mar 13, 2012 at 17:55
First, set your code aside for as long as practical. Work on something else, some other piece of code. Even after a day, you will be amazed at what you will find.
Second, document your code. Many programmers hate to document their code, but make yourself sit down and write out documentation, how to use the code and how it works. By looking at your code in a different way, you will find mistakes.
It has been said that true mastery of a subject is the ability to teach it to someone else. With documentation you are trying to teach someone else your code.
Transform this debugging technique to a code review technique: http://en.wikipedia.org/wiki/Rubber_duck_debugging
The concept works wonders for putting you into a proper mindset for working through code as if it were new.
-
3I believe the duck technique has been independently invented at multiple sites; here's a great story about it: hwrnmnbsol.livejournal.com/148664.html Commented Mar 12, 2012 at 22:20
-
10These days, my rubber duck is the Stack Exchange ask-a-question form. The desire to write a good question does the trick. Commented Mar 13, 2012 at 11:22
-
Excellent advice. It's even better as I already have a rubber duck on my desk (it's been here as a model for one of my game characters, but I guess it won't mind additional job of an IT consultant). Commented Mar 14, 2012 at 8:42
-
5@KevinReid, I would love to see some stats on abandoned SE posts - especially ones that people have been typing for longer than 60s on. I know I've done that same thing myself at least 5 times. Commented Apr 25, 2012 at 16:32
-
Wah! I didn't know this was " a thing". I just commented above that my comp sci professor recommended this during our first ever lecture, decades ago. He recommended a cat, but I suppose ea rubber duck would do. One thing is sure, it doesn't work without the anthropomorphic sidekick :-)– MawgCommented Dec 5, 2014 at 15:45
In addition to the useful tools mentioned in other answers, I think modifying your mindset it useful when doing a code review. It's silly, but I say to myself: "I'm putting on my code review hat". I do the same with QA.
Then it's important to limit yourself to that mindset. You're either the reviewer or the reviewee, you can't be both at once. So as a reviewer, I take objective notes to share with the reviewee. I don't change the code while I'm reviewing it, that's not something a reviewer should do.
The formality feels a little absurd at times, but I find when working solo that I'm often pulled in a lot of directions. So I may not necessarily close the review loop before something else comes up - that formality (and really, I'm talking rough notes in a wiki tool), is useful for making sure the review gets done. Likewise with my QA hat on, I add tickets for bugs before I fix them.
-
-
4@VJovic - I don't think I perform the best possible code review on my code, but I usually find things that could be improved. I read a lot of other people's code too. My viewpoint on what "good" code looks like is constantly evolving. I'm embarrassed by code I wrote years ago. It's no different than proofing your own article - it takes practice and a lot more effort, but it's doable. The key thing I can't review myself is if an abstraction makes sense to someone else. But I can ask myself how to make something simpler, is this necessary, etc. Commented Mar 12, 2012 at 14:08
-
@VJovic - As ThomasOwens mentioned, you can also create a checklist from past mistakes and run through that pretty objectively. That's the other nice thing about being sort of formal about it, you can see what you missed during the review and adjust your process accordingly. I find I learn a lot of lessons by tracking myself and making an effort to change my approach when indicated. Commented Mar 12, 2012 at 14:12
-
3Getting into the right mindset is really important. I find that it helps if I actually print out the code and go through it on paper with a marker pen. Then I cannot change anything when reviewing (which prevents me from going into coding mode) and can easily scribble comments and movement arrows on the paper.– LeoCommented Mar 12, 2012 at 14:34
-
That means reviewing old code, and not new code. For that you need to gain experience, which can take long.– BЈовићCommented Mar 12, 2012 at 14:56
It seems the common sentiment is that self-review is not effective. I disagree, and I think self-review can catch a lot of issues if done thoroughly.
Here are tips from my few years of experience:
- Have a rough checklist handy. These are things you want to flag while you read your code.
- Take your code review offline. It might sound wasteful, but take printouts that you can annotate and flip back-and-forth, or the digital equivalent of nicely highlighted pdfs synced to an iPad which is then taken offline. Go away from your desk, so that all you do is review your code distraction-free.
- Do it early in the morning, rather than the end of a working day. Fresh pair of eyes is better. In fact, it might help to have been away from the code a day before reviewing it afresh.
Just an FYI - these guidelines were part of recommendations in Oracle a few years ago when I was working there, where the aim was to catch bugs "upstream" before the code went into testing. It helped a lot, although it was considered boring job by a lot of developers.
-
3I'd also add "wait 24 hours" so you aren't looking at code you just wrote. Make sure it's at least 1 day old so you are seeing it after sleeping overnight and not touching it for 24 full hours. Commented Mar 13, 2012 at 7:54
-
I often use printouts when I need to review or especially refactor some code. It works wonders for me. Commented May 13, 2012 at 3:31
-
Like in some film we learned from GB that a fake orgasm is better than no orgasm -- self-review is better than nothing. Yes, you can train to do rubberducking a lot. But it's still quite ineffective compared to actual peer review. especially without being exposed to actually good reviewers for some time to pick up methods. Commented Jun 6, 2013 at 3:43
The Personal Software Process technique for reviews might be useful, although it relies on having historical data about your work and quality of products.
You start with historical data about your work products, specifically the number and types of defects. There are various methods of classifying defects, such as this one from a PSP course. You can develop your own, but the idea is that you need to be able to tell what mistakes you are making along the way.
Once you know what mistakes you are making, you can develop a checklist that you can use during a review. This checklist would cover the top mistakes that you are making that you think can best be caught in a review (as opposed to using some other tool). Every time you review a work product, use the checklist and look for those mistakes or errors, document them, and fix them. Periodically revise this checklist from time to time to make sure you are focusing on real, relevant problems in your code.
I would also recommend using tool support when it makes sense. Static analysis tools can help find some defects, and some even support style checking to enforce consistency and good code style. Using an IDE with code completion and syntax highlighting can also help you prevent or detect some problems before you click "build". Unit tests can cover logic problems. And if your project is sufficiently large or complex, continuous integration can combine all of these into a regularly-run process and produce nice reports for you.
Working solo means that unless you trust complete strangers to review code on your behalf, that you will need to look at the way you write your software in order to maintain code quality.
First and foremost, you should have a means to ensure that your code is matching requirements, and second that your code will be relatively easy to change if you decide later that you got something wrong. My suggestion would be to apply a Behaviour Driven Development approach for the following reasons:
- BDD means writing code test first. This ensures all of your code is covered by tests.
- BDD is essentially TDD, but with a slightly different focus and "language". What this implies is that you continuously refactor your code as you are working on it, and use your tests to ensure your refactoring efforts continue to ensure that your code satisfies your product spec.
- BDD language encourages tests to be written in the form of statements that essentially encode requirements as unit tests.
So the idea here, is that your continuous refactoring of code even after you get your tests to pass, means that you are effectively reviewing your own code and using your unit tests as the "extra pair of eyes" which make sure your code doesn't stray from the requirements that are encoded in the tests. Also, high test coverage based around requirements ensures you will be able to change your code in the future without failing the requirements.
The real issue for you will be whether or not you're able to spot potential issues in your code that will indicate a need to refactor. There are several profiling tools on the market that can help you with this, as well as several other tools which are concerned with code quality metrics. These can often tell you many things that code reviews can miss, and are a must when developing projects on your own. In reality however, experience is the key, and once you are in the habit of being merciless in your refactoring, you will likely become much more critical of your own code. If you haven't already, I'd suggest reading Martin Fowler's Refactoring book as a starting point, and looking for a good BDD API that you feel will work for you in whichever language you have chosen to work with.
Whenever I've been in the same situation as yourself, I've tried to solve the problem of "being too close to the code to objectively examine it" by using code review / metric tools. It goes without saying that a tool cannot give the same value as an experienced reviewer, but you can still use them to pinpoint areas of bad design.
One tool that I've found fairly useful in this regard was SourceMonitor. It's a bit simplistic, but it gives a good mid-level opinion of your code, such as the number of methods in a class, and the complexity of each method. I've always felt that this type of information was as important (if not more important than) the enforcement of coding styles via tools like StyleCop, etc (which are important, but are often not the source of the largest problems). Use these tools with the usual disclaimers: know when to break a rule of thumb, and something that is all green in a code metric tool isn't automatically of good quality.
I can't tell you the number of times I have been explaining something to a code reviewer and the lightbulb in my head turns on and says, "Hey wait a minute." So I often find my own mistakes in the code review that the other person didn't see. So you could try that, just start explaining the code as if there was a person sitting next to you who was trying to understand what you did and why.
Another thing that I find frequently in code reviews is that the developer did not actually follow the requirement. So comparing your code and what it does the the actual requirement is a good check.
We frequently do things like SSIS packages that have similar structural needs - for code reviews I developed a checklist of things to check (is the configuration correct, is logging set up, does it use the metadata database, are the files in the standard location, etc.). You might have some things that would be handy to check every time in a code review as well. Sit down and think about what you would put on a checklist of things you want to make sure to check in your code review (First item, make sure the requirement is met, next item might have something to do with trapping and logging errors). As you make mistakes and correct them, you may add other items to the list (say something like, do I move to the next record in a loop or am I going to endlessly repeat the same first item - it only take one endless loop to teach you to look for that!). This is mostly to keep you from forgetting to do some things that should be done.
-
1As Patrick Hughes suggests in his answer, using a proxy like a rubber duckie to stand in for the reviewer helps the mindset. Commented Mar 12, 2012 at 22:21
Give it 3 months, then go back and look at your code. I promise you, if you can't find something wrong with it (or question who wrote this junk!) you are a better man than I!
-
That's my technique also. 3 months is long enough that anything I can't immediately understand needs to be simplified or documented better, but short enough that I still remember what's going on enough to easily rectify it. Commented Mar 15, 2012 at 20:50
I usually print out all my code and sit down in a quiet environment and read through it, I find a lot of typos, issues, things to refactor, cleanup by doing that. It's a good self-check that I think everyone should do.
-
A good addition to the advice above, thanks — although I think that a tablet or something like that (with editor, but without a development environment) would work too. I wonder who downvoted that and why. Commented Mar 14, 2012 at 8:40
Back in college I was a writing tutor. It has certainly given me some perspectives on coding that I think many developers would never have thought of. One of the most important is to read your code aloud. It doesn't sound like much, but I will give a perfect example that I think everyone can relate to.
Have you ever written an email or a paper, reread multiple times to make sure its correct, then sent it, only to find that you have a glaring spelling error, typo or grammatical error? I just did this yesterday when I asked a client to press the shit key instead of the shift key. When you read in your head - you see what you want to see.
This is a shortcut for the 'just wait a day or a week or a month' suggestions others have made. If you read it aloud you catch the same things. I don't know why it is so effective, but after sitting with hundreds of students and having them read aloud, all I can say is that it works.
-
+1 This would go along with the "explain it to your cat" approach. Using different parts of your brain can be helpful when you can't use a co-worker.– BMitchCommented May 14, 2012 at 15:17
-
Most of the people tend to consider their code as their own babies and feed them with ego to rather than reality. Just like any other code reviews, review it as you're seeing someone else's code. Completely forget that you've wrote something. Review each line of the code. A check list would helpful for being aesthetics about reviewing own code. Automated tools for code review can help to some extend. I have used some tools like klocwork (commercial software), This is quite useful while you're working in large projects and several developers are working for it. Always focus on defect detection rather than correction.
But a best practice would be, review yourself and later involve at least two other people for review with distinguished roles.
Consider doing a Fagan inspection by yourself - you'll have to adapt the process because you're by yourself, but you should be able to get quite a bit of value out of it. The trick is going to be to find the right "ruleset" to evaluate your code against as a solo dev, and then having the discipline to ask those questions in a critical, analytical, merciless frame of mind each time. I suspect you may want to brainstorm your own 4-5 crucial questions to start with, and then evolving it over time. Some people are against formal inspections because they seem to be so time-intensive... before you decide that they're too expensive, bear in mind all the statistical evidence that doing inspections properly actually reduce project time. Here's a Wikipedia link that you can start further research with:
http://en.wikipedia.org/wiki/Software_inspection
There have been a few books too, e.g. Google for "Software Inspection Process" by Strauss and Ebenau.
Another option is to pay someone to inspect an important project - or perhaps pay them occasionally to do inspections of all your code. This guy's pretty good, we've flown him out a number of times to train our newer devs:
Apart from all the recommendations for code review, you can use the tools like PMD and findBug to do first level of sanity for your code.
This has not actually been placed in an answer yet (but has been added as a comment to an existing answer)
Review your code after a good nights sleep, e.g. start the day by reviewing the code that you wrote the previous day.
This will of course not give you the collective experience of a team, but it will give let you review the code from a new perspective.
For instance, if you left a piece of code with a nasty hack, you might not be too inclined to fix it up, if you review your code immediately after. After all, when you start reviewing your code, you already know, and have accepted, the presence of this hack. But if you have had a good nights sleep you are probably more motivated to find a better solution.
When we sleep, the brain doesn't actually stop working on the problems that we have, so you might actually come up with a solution there, though those solutions may sometimes present themselves in odd ways.