Open
Conversation
SebaMutuku
suggested changes
Dec 8, 2022
Contributor
SebaMutuku
left a comment
There was a problem hiding this comment.
Please fix the codacy issues and add tests for the functionality .
Ensure to cover the exception block
Author
|
Hello @SebaMutuku thank you for the review. I have updated the code to fix Codacy issues. I have also integrate Codacy to Indonesian local repository for more linear and faster future reviewing process. I will submit the PR once unit testing is done. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request provides fix for Issue #924.
The Problem
Method
getPreviousContactFactsinPreviousContactRepositoryclass has unhandled recursion that caused the ANC app to crash when it couldn't find previous contact data. For more detailed problem and analysis, please refer to the Issue #924 page.Solution
Instead of calling a method recursively, this fix provides a solution to limit the previous contact data searching while still answers the requirement of returning the latest referral contact (with negative contact number) or immediate previous contact. That way, memory leak problem that caused the app to crash can be prevented.
What's Changed
To get immediate previous contact, this fix use two methods:
getPreviousContactFactswithoutcheckNegativeparametergetPreviousContactFactswithcheckNegativeparameterThe first method does the actual search. It will search the contact based on
contactNoparameter that can be positive or negative. So it can return both previous contact and referral contact based on the parameter.The second method is the method that handles the request from other classes. It has the
checkNegativeparameter to do the referral contact searching when needed. If the providedcheckNegativevalue istrueit will call the first method to search previous contacts withcontactNofrom-10to-1, before searching for the currentcontactNovalue.Obviously, the search limit can be changed by modifying the value of
negativeContactsLimitonPreviousContactRepository.javafile line421. If you are going to modify the limit, please enter a positive number (number10to search referral contacts from-10 to -1.Because this doesn't change the input, output, and how the method is called, this fix doesn't need any adjustments in other classes.
Notes
CC: @Naima-Bashir @dubdabasoduba