4 ms·
I've been reviewing code for a long time, and I've had lots of my own code reviewed. You have to learn to accept code reviews as mentoring and learning and stri
by gregjor 10y ago
I've been reviewing code for a long time, and I've had lots of my own code reviewed. You have to learn to accept code reviews as mentoring and learning and striving for consistency across the team. If you think of reviews as a contest of egos or a test to pass you aren't getting the most out of it. Most juniors now days don't get the opportunity to have a more experienced programmer review their code.
Main problems I see when reviewing code:
1. Doesn't meet requirements, or requirements not understood. If you aren't clear on what you're supposed to deliver before you start coding you need to ask questions. Make a plan, identify intermediate goals, have someone else validate your approach.
2. Over-engineering. Solve the problem assigned. Don't create more problems or try to plan for every eventuality. Prefer simple, understandable solutions.
3. Not tested. Test carefully. Then test again. I don't mean just making all the lights green in TDD. You should go test carefully and make sure you see expected behavior according to requirements. Think about edge cases. Build testability, logging, etc. in from the start.
4. Sloppy work. Read your code. This is called desk checking. Review your code before someone else does. It's surprising how many obvious errors you can find just reading through code.
5. Idiosyncratic style. If your team has written standards, follow them. If not propose that the team define and follow some standards. In lieu of that make your code look like what the senior devs produce. Lots of time is wasted resolving disagreements over things like indenting and variable naming.
If you have the luxury of code reviews keep notes about what your reviewer asked you to change. Determine if the issues are style, efficiency, lack of testing or sloppy work, or misunderstood requirements. Change your approach, and talk to the team if you think there's something everyone can do to address the issues.