Conversation
Changes applied by IntelliJ "cleanup code" action. It adds `final` keyword where it's apllicable and simplifies boolean expressions.
They don't change after initialization so let's keep them outside of the constructor to reduce its size.
`getStoredQueue` method could never return `null` so we mark this method as `@NonNull` and remove all nullability checks to improve code clarity
| @NonNull | ||
| private ArrayList<Map<String, Object>> getStoredQueue() { |
There was a problem hiding this comment.
getStoredQueue could never return null. Look lines 525-528. Also, IDE was smart enough to infer it
That's why I explicitly added @NonNull annotation.
There was a problem hiding this comment.
Praise (❤️): Kudos on making this change, it removes lots of unnecessary nullability checks! 🥇
There was a problem hiding this comment.
LGTM, thanks for this PR @wzieba ! 💯
I have left a few suggestions (💡) and one minor (🔍) comments for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.
EXTRAS:
- Suggestion (💡): Consider removing the unnecessary
url == nullcheck from thetrackPageview(...),startEngagement(...)andtrackPlay(...)methods. PS: Same applies to thevideoMetadata == nullcheck. - Minor (🔍): Between commit 94a551e and c51a206, the later had extra changes related to the former on
QUEUE_SIZE_LIMITandSTORAGE_SIZE_LIMITwithindoInBackground(...). This made both commits harder to review in isolation. Consider rebasing next time to fix that and have each commit specific to the change it introduces.
| private static final String STORAGE_KEY = "parsely-events.ser"; | ||
| // emulator localhost | ||
| // private static final String ROOT_URL = "http://10.0.2.2:5001/"; | ||
| private static final String ROOT_URL = "https://p1.parsely.com/"; |
There was a problem hiding this comment.
Suggestion (💡): Consider using the below shorter alternative:
private static final String ROOT_URL = "https://p1.parsely.com/"; // emulator localhost: http://10.0.2.2:5001/
parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java
Outdated
Show resolved
Hide resolved
parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java
Outdated
Show resolved
Hide resolved
|
👋 @wzieba ! I just reviewed the new changes and everything LGTM, thank you so much for applying those suggestions, this class is getting cleaner and cleaner! 🙇 ❤️ 🚀 Feel free to merge this when you are ready! ✅ |
2 participants
Description
This PR contains a set of code quality improvements: removing unneeded checks or making fields
finalwhen there's no need for them to be mutable. It's not the complete set of improvements I could provide, but I try to limit the size of PRs.