Did you know that Android Studio has static code analysis built in? You can run this tool using the inspect code dialog. When launched you can specify what you want to analyze; your whole project, individual modules, or just a directory.
Fire it up and go fix a snack and prepare a warm beverage, it will take a few minutes to run. The results will start to be populated and categorized within an "Inspection Results" window. You could potentially see a lot of groups, let's focus on; Android, Kotlin, Java, XML. Each of these groups has organized rule sets and displays the list of files that violate them.
For example, if we had layouts that used left/right instead of start/end. They would appear in the section Android -> Lint -> Internationalization -> Using left/right instead of start/end attributes.
As you dig into each file you will see that sometimes Android Studio is able to resolve the issue and offers a fix option in addition to suppression.
That's really powerful! You may be tempted to get started fixing a whole bunch of things! Wait a moment though, it's not as easy as you think.
Not So Fast!
We need to be thoughtful in how this is done in order to minimize risk and maintain developer sanity. We don't want to introduce bugs or have to push out a hotfix because of a well intentioned change. Likewise, we want to keep our changes appropriately sized so our peers will still be our friends after reviewing any changes we submit.
Changes of Appropriate Size
Keeping your changes per pull request appropriately sized will go a very long way in minimizing risk and keeping our peers sane. A good start is keeping them as small as possible. This allows the reviewer to scrutinize the changes in greater detail and hopefully catch any issue if present. This is especially important if the changes are high risk and/or the modification is hand done.
There are times when larger changes are allowable. For example; removing unused imports. We don't really want this spread over many change requests. It's ok if this touches lots of files and is larger than normal because it is low risk. We are just removing unused imports and probably relied on Android Studio to modify the files.
Group Like Changes Together
A really great and simple technique to help minimize risk is to group like changes together. It makes it easier for the reviewer to spot any issues because all the changes should be similar.
If the changes are small and you don't want to make separate change requests then you can organize your changes across commits.
Test It Out
We need to test out our changes and make sure we haven’t broken anything in the application. Performing some due diligence here will go a long way in helping to minimize the risks. Keeping our changes appropriately sized and grouped together will help us identify a problem faster if something goes wrong.
Effectively Communicate Changes
Summarizing and explaining your changes in the pull request will help the reviewer understand what they are reviewing and why. If you are particularly concerned, it's always good to say so. "This change was refactored by hand, would you mind looking it over thoroughly?"
Merge Changes After A Release
We should consider when we submit our changes for inclusion. To help minimize risk we shouldn't do this before a release. Ideally, we should aim to have our changes merged soon after our production build has been pushed. That will give us the most amount of time to test our changes before the next release.
We are asking a tremendous favor from our colleagues to review this code. They will undoubtedly have questions. Be patient and listen to your colleagues’ concerns and answer any questions they may have. You may have to revert some well intended changes or iterate further with your teammate. This is all part of the process and is expected. Do your best and always try to lean on the side of minimizing risks.
Understand the Risk
Understanding the risks and minimizing them will help reduce the chances of introducing bugs. Our changes and their impact on the code base needs to be understood for us to effectively assess that risk. Once we know that, we can apply the techniques above to help minimize those risks.
You will break things, be prepared to fix them and own up to any issues found.
- Changes should be of appropriate size
- Group like changes together
- Test It Out
- Effectively communicate with your team
- Merge your changes after a release
- Be patient
- Understand the risk and minimize for it
Why I Love It
It's a choose your own adventure style story where you can decide your path whatever that may be for a day. Want something easy and straightforward? Pick a low risk issue that is easily understood. Want a challenge? Dig for esoteric issues that require architecture changes. Maybe, you discover a utility class that lacks test coverage. Adding in unit tests for that utility could also be the adventure!
The end result will be more than just the simplistic reduction in lint warnings. Your journey will take you all over the code base to places you would never expect. You might find yourself checking out a subsystem or file that you have never looked at before. You will also learn about types of issues and apply this knowledge during code review.
We become and help others to become better programmers by finding and fixing issues.
Some Issues To Check Out
Now that we’ve covered strategies to minimize risk I would like to part ways by providing some starting places for you to continue the journey. Below is a small list of some common issues that can be corrected automatically. Happy hunting!
1) Class member can have private visibility - This inspection catches when we can make our instance variables or functions private. They can be found in the Inspection Report
Kotlin -> Style Issues -> Class member can have private visibility
2) Might be 'const' - This inspection catches top-level val that can be declared as a const. They can be found in the Inspection Report
Kotlin -> Style Issues -> Might be 'const'
3) Remove redundant qualifier name - This inspection catches redundant qualifier name usage. For example referencing something using a fully qualified name may not be necessary due to a static import. They can be found in the Inspection Report
Kotlin -> Redundant Constructs -> Remove redundant qualifier name
I hope you enjoyed this article, please feel free to reach out to me on Twitter.
Interested in working on a small team pursuing a number of great new practices like this? We're hiring!