Fixes #71 Implement Analytics class #72
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #72 +/- ##
============================================
- Coverage 62.21% 59.43% -2.78%
Complexity 442 442
============================================
Files 84 85 +1
Lines 1736 1817 +81
Branches 166 178 +12
============================================
Hits 1080 1080
- Misses 611 692 +81
Partials 45 45 ☔ View full report in Codecov by Sentry. |
| * @param days The number of days to add. | ||
| * @return The new Date object. | ||
| */ | ||
| public static Date addDay(Date date, int days) { |
There was a problem hiding this comment.
This is not urgent, but I think we can refactor date representation in our project to use LocalDate or LocalDateTime. It is a more recent API that has more inbuilt methods such as plusDays() which can be used to avoid weird methods such as this,
| private int numOverdueLoans; // total number of overdue loans | ||
| private int numActiveLoans; // total number of active loans | ||
|
|
||
| private float propOverdueLoans; // proportion of loans that are overdue over active loans |
There was a problem hiding this comment.
Comment does not match implementation, which suggests it is proportion of overdue loans over all loans.
| @@ -57,6 +57,10 @@ public Address getAddress() { | |||
| return address; | |||
| } | |||
|
|
|||
| public Analytics getAnalytics() { | |||
| return Analytics.getAnalytics(loanRecords); | |||
There was a problem hiding this comment.
It seems that each time the analytics are viewed, it is reconstructed. Is it possible to refactor this in the future such that it is maintained and modified?
There was a problem hiding this comment.
It's possible, but it's a trade-off between memory and runtime.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "Number of loans: " + numLoans + "\n" |
There was a problem hiding this comment.
Maybe StringBuilder will be more appropriate since this is a significant number of strings?
There was a problem hiding this comment.
This is fine, it's standard to just add strings like this since this is a constant operation.
| analytics.updateDateFields(loan); | ||
| } | ||
| analytics.updatePropFields(); | ||
| analytics.updateAverageFields(); |
There was a problem hiding this comment.
It seems that the update methods must be called in a specific order to prevent errors. While it is not a concern now because each Analytics object is effectively immutable and getAnalytics() is the only way of obtaining it, it would be good if this can be addressed so that future iterations do not run into related bugs.
narwhalsilent
merged commit ccf4da6
into
AY2324S2-CS2103T-W13-1:master
Minor changes to Person class:
Added
getAnalytics()method.Minor changes to DateUtil class:
Added
addDay(Date date, int days)method.Minor changes to Loan class:
Added
isOverdue()method.New Analytics class:
This class handles the analysis of a LoanRecords object.
This class can only be instantiated by calling the static method
getAnalytics(LoanRecords loanRecords).You should not be calling this method directly, instead you should call the
getAnalytics()method from the Person class.This class contains the following fields:
numLoans: total number of loansnumOverdueLoans: total number of overdue loansnumActiveLoans: total number of active loanspropOverdueLoans: proportion of loans that are overdue over active loanspropActiveLoans: proportion of loans that are active over total loanstotalValueLoaned: total value of all loanstotalValueOverdue: total value of all overdue loanstotalValueActive: total value of all active loansaverageLoanValue: average loan value of all loansaverageOverdueValue: average loan value of all overdue loansaverageActiveValue: average loan value of all active loansearliestLoanDate: earliest loan date of all loansearliestReturnDate: earliest return date of active loanslatestLoanDate: latest loan date of all loanslatestReturnDate: latest return date of active loans