How do I thoroughly review my own code?
I have been seeking to develop habits / follow a system to increase code quality, both in terms of readability and number of functional issues. I have come up with a few ideas I have listed below but I am curious what other people have been trying out! Have any of the below worked for you? What specific steps do you take to ensure code quality?
- More awareness while writing code: it's tempting to write a rough draft version of the code thinking you will polish it later but oftentimes it's easier to spot mistakes while your attention is on the erroneous line of code. Example of this in action: search for where the method you're modifying is used in the code base before making changes to it.
- Do not submit the code for review immediately after writing the code. Give yourself a short break, come back and review your code with a fresh mind, which will make spotting bugs easier.
- Write down the possible edge cases in a notebook before submitting the PR for review. For medium to large PRs, the process of writing itself can help you consider more edge cases than simply keeping them in your head.
Discussion
(4 comments)- 19
9 months agoAndroid Engineer @ RobinhoodThe general steps I follow are:
- Write small pull requests. Smaller pull requests are much easier to read for everyone, making the overhead with interacting with the diff lower. Reviewers can provide much more narrow feedback, you're much more easily able to write focused tests, and it's much easier to infer context from the code.
- Write detailed summaries on the pull request. This usually varies based on what the pull request is for, but I do a mix of: linking project specs (and designs if it's for the frontend), writing a test plan, explaining how I arrived at the current code for the diff.
- For any pull requests that touch code, write a test plan on how you tested the code (manually, existing tests, new tests, etc.). If you cannot perform any sort of testing, explain why the changes can't be tested.
- 23
9 months agoMid/Senior Software Engineer+1 for Jonathan's suggestions!
Another thing I do when I write code is think about how future versions of this code could potentially look like. For example, if I'm implementing a new feature that may be extended or modified in the future, how easy would it be for that new change to be made? How could these potential future changes affect the rest of the system (in terms of backwards compatability, performance, readability, etc)? The answer to these questions not only give a lot of information in how my code should be structured, but they can also guide your solution to be as future-proof as possible.
Also, as a general tip, I structure most of my PRs to have these sections:
- Context - includes description of the problem, solution, and alternative solutions considered
- Description of Changes - a summary of the actual code changes made in the PR
- Before/After Screenshots (if applicable)
- Test Plan - unit tests, integration tests, manual testing, etc
Explicitly writing these out in a PR description helps make sure I - and everyone reviewing my code - clearly understands the context and thought process behind the code changes.
- 11
9 months agoTech Lead, Senior Software Engineer [L5] at GoogleMelissa and Jonathan already did a great job (I’m especially a fan of small, atomic cl/pr/diff!)
I would add that if your team has an agreed upon style guide or something similar, attempt to apply that onto your cls. If you don’t really know how to do this or your team doesn’t have a style guide, try the following instead:
Look through your past changes and the comments your reviewers left. Do you observe a pattern of similar comments? (Is it… lots of naming adjustments, unit test structure, or something else?) Focus on getting better at a specific set of feedback and ask yourself if your new cl has similar problems to prior ones. In general, try not to have your reviewer point out the same problem 3 times in a row, and past comments are a great source of learning about what kind of mistakes you tend to make.
- 179 months agoTech Lead @ Robinhood, Meta, Course Hero
The 3 items you have listed are spot-on 👏 . If you implement all of those, you will get promoted from junior to mid-level in no time!
It's so hard sharing new insight on top of what's already in the original question and everyone else's great comments, but I was able to come up with this one: Look at other PRs in the same area and proactively learn from them.
So let's say you're submitting a commit to modify module X:
- Spend 15-30 minutes going through the ~5 other most recent code reviews that touched module X
- Analyze the code reviews that had rejections in-depth before they were finally approved. Absorb that feedback and understand why those commits were rejected
- Make sure that your own pull request doesn't do the things that got the other code reviews rejected
I recommend these resources as well to learn this concept and the others mentioned here more in-depth:
