Android Code Review Checklist
I came across Michaela Greiler’s Code Review Checklist earlier this year and it’s now one of my favorite and most frequently-used programming resources. However, it’s a high-level checklist and doesn’t include anything specific to a platform or a language. I decided to write an Android-specific one. It’s meant to be used as a supplement to Michaela’s rather than as a replacement. All her high-level points apply to Android development as well, but I’m adding additional points to check when reviewing Android code. There are also a few non-Android-specific points — mostly things to watch out for in existing code (eg. outdated code comments) or things relevant to general front-end engineering (eg. user-facing error messages), since the checklist doesn’t include them.
Implementation
Does the code follow the project’s architectural patterns?
- Eg. if you’re using MVP, business logic should be in a presenter class rather than an Activity or Fragment.
Does the code follow the project’s coding style guide?
- This could be a company’s internal style guide, Kotlin Coding Conventions, Google’s Java Guide, etc.
- Style guide-related issues shouldn’t come up too often during code reviews; if they do, you may want to update your lint configuration or Android Studio code style settings (under Preferences → Editor → Code Style ).
Is there any code that becomes obsolete after the code changes and therefore can be deleted?
- Eg. code behind a feature flag that has been launched.
- This is especially important for mobile since APK sizes should be kept small. If a large chunk of code across multiple files can be deleted, I recommend doing it in a separate pull request so that the deletion doesn’t distract reviewers from the feature implementation.
Error handling and logging
Are the error messages being displayed user-friendly?
- If there’s a designer involved, work with them to design error states.
- In some cases, it may be better to fail silently as long as the error is still being logged. Eg. if an image load fails, displaying an old cached version of the image rather than an error may be a better user experience.
- If it’s an error the user may be able to fix, are you letting them know? Eg. if a text input only support numbers Are remote non-fatal errors (sometimes called “soft crashes”) being logged?
- If you catch an exception and handle it, your crash reporting solution will no longer log it as a fatal exception, but it would still be useful to know how often it’s happening. Using Firebase Crashlytics as an example, you’d want to do something like this:
Should any code be separated into multiple lines for easier stack trace analysis?
- Android Studio’s Analyze Stack Trace tool lets you analyze any stack trace you paste in and point you to the line that caused the crash. If you have too much going on in one line, it may be hard to pin down which part of the line caused the crash.
- For example, the following line may throw an exception if some of the strings are null, or some of them aren’t valid representations of numbers, or none of them are greater than 10.
numberStrings.requireNoNulls().flatMap < it.toInt() >.first < it >10 >
The following refactor makes debugging easier:
What happens when there’s no network connection?
What happens when an API call fails?
What happens when there’s a slow network connection?
- You can test this on an emulator by going to its extended controls and choosing different network types and signal strengths.
What happens when the device is rotated?
What happens if some permissions that the feature requires are denied?
What happens if a user inputs something unexpected?
What happens when there are edge cases in the data?
- Eg. if the feature includes displaying a list, how does it look when the list is empty or very long?
Usability and accessibility
Most of these are only applicable if there are UI changes.
Disclaimer: I don’t do a thorough device size and display size check for every pull request, since they can get tedious and rarely reveal major issues. I recommend doing them for changes to critical flows (eg. login or payment). For less critical flows, it’s worthwhile to go through them once in a while on different device sizes and accessibility settings to make sure they still work as expected.
If you have designs provided by a designer, does the UI match them?
- Make sure you check the loading and error states too!
Are the correct view classes being used?
- Eg. if there’s a TextView , does your project have a custom extension of TextView that should be used instead? Or if the project follows Material Design, are you using a MaterialTextView ?
Do the views and styles follow the company’s design guidelines?
Are you using fonts and colors from the company’s branding?
How does the feature look on a small device?
How does the feature look on a tablet, if supported?
How does it look in dark mode, if supported?
- You can turn on the dark mode by going to Settings → Accessibility → Dark theme .
How does the feature look with non-default font and display sizes?
- To try it out, you can change these sizes on your device by going to Settings → Accessibility → Display .
Does the XML code support usability and accessibility?
- Eg. are string resources in the correct folder for internationalization?
- Eg. do EditTexts have the correct inputType ?
Testing and testability
Is the code testable?
- Ideally, the bulk of the business logic is in non-Android classes and therefore testable with simple unit tests.
Can classes that were added or modified be easily mocked?
If the project uses UI testing, should any new UI tests be added and existing ones updated?
Are there any tests that become irrelevant or inaccurate after the code changes and should be deleted or updated?
Performance
Are threads used correctly?
- Eg. long-running tasks like API calls and database queries should be deferred to background threads to avoid blocking and freezing the UI thread.
- Eg. threads are being cleaned up and not causing potential memory leaks.
Is there object allocation or other code in frequently-called functions that can be moved somewhere else?
- Eg. onDraw() can be called up to 60 times a second in the worst case, and initializing too many objects in it can cause dropped frames and a janky UI.
- If you have something like:
consider refactoring to this:
- Similarly, an adapter’s onBindViewHolder() may be called frequently, so any code that doesn’t need to happen on every bind should be moved to onCreateViewHolder() instead.
Are there any API responses that can be cached, in order to avoid high data usage and battery drain?
Are there any API requests that can be done less frequently?
Dependencies
This section is mostly relevant if the code changes involve adding new dependencies on external libraries or bumping an existing dependency by a major version.
How necessary is it? If the project only needs a small piece of the library’s functionality, would it be better to reimplement it yourself instead?
How much does it add to the APK size and is the size increase worth it?
How widely is the library used in the Android community and is it actively maintained? Who’s maintaining it?
- Ideally, it’ll be maintained by Google, a company with a good record of maintaining open-source projects (eg. Square), or an active open-source community rather than a single person.
Does Google recommend using the library or an alternative?
- If you’re not using the Google-recommended solution, make sure you can articulate good reasons for doing so. Google does change its mind a lot so recommendations should be taken with a grain of salt.
If there’s a version bump, does the new version introduce any breaking changes?
If the project uses Proguard or some other code obfuscator, does an obfuscated build of the app still work as expected or do the obfuscation rules need to be updated?
- A lot of apps have Proguard enabled for release builds only, and testing on a debug build isn’t enough to reveal Proguard issues.
Readability
Are there any places where lambda parameters should be declared explicitly?
- Kotlin’s scope and collections functions are great for expressiveness and conciseness, but it’s easy to go overboard with conciseness and end up with confusing code. If there are many it s in the same block of code, it can be hard to understand what a particular one is referring to and what the block is actually doing. Naming parameters is useful when there are nested scope functions ( let , run , with , apply , and also ).
would be more legible with a refactor to the following:
- They’re also useful for chained calls on a collection to make it clearer what the parameter is exactly at different stages of the call chain.
would be more legible with a refactor to the following:
- Eg. when there are a multiple resource Int s, annotations help clarify which type of resource each one is:
Do the code changes follow the project’s naming conventions?
- Eg. if the existing view model classes are all named SomethingViewModel , any new view models added should follow this pattern.
Are there any places where using named function arguments would be helpful?
Are there any places where explicit types would be helpful even if the compiler doesn’t require them?
- Eg. if you have something like val title = getTitle() , even though the compiler can infer title’s type, it may be unclear to another developer whether it’s a String , String? , or @StringRes Int . By specifying the type explicitly like val title: String? = getTitle() , you save other developers from having to look at getTitle() ’s function signature.
Are there any places where explicit types are used unnecessarily?
Are there comments that should follow documentation engine formats like Javadoc or Dokka ?
- Eg. if you see a plain comment like:
you may want to update it to the following:
Are there any comments that are no longer accurate after the code changes and should be updated?
Final words
As a meta-comment, if time permits, I like to let a code review sit overnight as a draft and review it myself the next morning with fresh eyes before officially requesting a review. You’ll get a better sense of what it would be like to see your changes as someone reading the code for the first time. You’ll also likely catch minor issues that you didn’t see the day before.
Here’s Michaela Greiler’s Code Review Checklist again, which I highly recommend reading in addition to this article.
And here are some additional code review resources I like:
- How to Do Code Reviews Like a Human: tips for the human/social part of code reviews
- How to make your code reviewer fall in love with you: best practices for putting up a code review
- Android Code Review at Yalantis: blog post about Android code reviews that does a deeper dive into some of the points I mentioned
Happy code reviewing!
Thanks to Russell , Daphne , and Kelvin for their valuable editing and feedback ❤️
Источник
Detecting Incoming Phone Calls In Android
To download the code, login with one of the following social providers.
Login Login 
Be patient. we are fetching your source code.
Objective
Are you trying to detect incoming call and incoming call number in your Android app?
Are you facing problem in detecting whether your phone is in ringing state, receiving state (off hook) or idle state?
Do you wish to take any action when there is an incoming call or when the phone goes into off hook state (Phone goes into off hook state when you receive a call) or when the phone goes into idle state (when you cut your call)?
I was working on a pretty huge Android project recently. In that I had to detect the state of the phone.
So if you wish to know how I achieved that, read on further.
DETECT PHONE CALL STATE EVEN YOUR APPLICATION IS CLOSED
Do you know, even when your app is closed you can detect phone call states in your Android phone from within your app?
Cool…right? Now lets concentrate on «How to do» that!
RECEIVERS ARE THE KEY
Have you heard about receivers in Android?
If yes, then learning the phone state concept would be pretty easy for you
And don’t worry if you are not familiar with receivers, I will tell you what actually receiver is and how to use it in our app.
WHAT THE HELL ARE RECEIVERS?
Broadcast receivers help us to receive messages from system or other applications.
Broadcast receiver respond to broadcast messages (intent, events etc.) from system it self or other applications.
To know more about Broadcast Receiver, follow below link:
To implement Broadcast Receiver in our app, we have to follow below 2 steps:
- Create Broadcast Receiver
- Register Broadcast Receiver
Lets first create a simple project in Android Studio with blank activity.
If you are new to Android studio and don’t know how to create new project, please check the following link:
LETS CREATE & REGISTER A BROADCAST RECEIVER
Create one Java class named PhoneStateReceiver and it should extend BroadcastReceiver class.
To register Broadcast Receiver, write below codes in AndroidMainifest.xml file.
You must write these lines between tag.
Our main goal is to receive mobile phone call state. So for that we have to define android.intent.action.PHONE_STATE as an action.
Now your AndroidMainifest.xml file should look like below:
Voila! We have successfully integrated Broadcast Receiver to our project.
HAVE YOU TAKEN PERMISSION FROM USER?
In order to receive phone call state in an application, you have to take permission from the users.
To take permission, we need to write below line in our AndroidManifest.xml file.
Now your AndroidManifest.xml file should look like below:
THE STORY OF onReceive() METHOD
Now lets get back to our PhoneStateListener class, where we have extended BroadcastReceiver.
In this class, we must override onReceive(Contex context, Intenet intent) method because it’s an abstract method of BroadcastReceiver class.
Do you have any idea about onReceive() method?
If I ask you to take a wild guess about what the method should be doing, what your guess would be?
Hint: The name itself is self-explanatory.
Guess… Guess… Guess.
Yes, you have done it. Your thinking is totally right that onReceive() method receives each message as an Intent object parameter. We have already declared Broadcast Receiver & registered it in AndroidManifest.xml .
Now, let’s take a look at PhoneStateReciver.java file and specifically we would be focusing upon onReceive() method of that class.
We are done with lot of stuff. Do you think now we would be able to detect phone state?
Right now, whenever a phone call comes we’ll get a toast message that says Receiver start and we’ll also be able to see that in our console since we have also printed the statement on the console.
We won’t be able to identify the different states of the phone. Our objective was to find the states like:
KEEP CALM AND KEEP IT UP TO DETECT PHONE STATES
What we have to do to detect all phone states? Do you know about Telephony Manager in Android?
Don’t worry if you are not familiar with Telephony Manager. I’ll guide you what is Telephony Manager and how we can use it to detect phone call states.
Telephony Manager gives you all the states information of Android devices calls. Using these states, we can preform various actions.
If you wish to learn more about Telephony Manager, please go through below link:
We can detect our phone call states using TelephonyManager.EXTRA_STATE . It indicates the current call state and it will return phone state as a String Object.
So declare one String object like below and get different phone call states:
To get different phone call states, we have to implement our code like below:
So now our PhoneCallReceiver class looks like below:
YES, FINALLY WE DID IT.
We have successfully implemented all the things. You can check using emulator as well as installing app into your device.
If you don’t know how to check in emulator using device monitor, please follow below steps:
- Start Android studio
- Click on Android Device Monitor. See below screenshot if you can’t find Android Device Monitor.
Follow below screenshots to get more idea:
If you are using new version of Android Studio (2.1 +) or if you have latest HAXM then you have to follow screenshot steps:
That’s it… You are finished with all the things and now you can detect all the phone states using emulator. You can find outputs like below screenshots.
OUTPUT 1. INCOMING CALL STATE DETECTED
OUTPUT 2. RECEIVING CALL STATE DETECTED
OUTPUT 3. IDEAL STATE DETECTED
We’re done with our main goal of detect phone states.
NEED INCOMING CALL NUMBER?
Have you read Telephony Manager class in detail?
Have you seen TelephonyManager.EXTRA_INCOMING_NUMBER ?
If you are already aware about TelephonyManager.EXTRA_INCOMING_NUMBER , that’s good it means you already read my given link of Telephony Manager class.
TelephonyManager.EXTRA_INCOMING_NUMBER returns an incoming call number as a string.
If you wish to detect incoming call number in your app then you can do so using below code:
Yupii. We have successfully detected Incoming call number.
I hope you find this blog post very helpful while with Detecting incoming phone calls concept. Let me know in comment if you have any questions regarding Detecting incoming phone calls. I will reply you ASAP.
Learning Android sounds fun, right? Why not check out our other Android Tutorials?
Got an Idea of Android App Development? What are you still waiting for? Contact Us now and see the Idea live soon. Our company has been named as one of the best Android Application Development Company in India.
Parimal Gotecha
I am 3D game developer with an aspiration of learning new technology and creating a bright future in Information Technology.
Источник
