Open lionscribe opened 11 years ago
nelenkov
commented
11 years ago What version are you using? In 2.6.0 at least the first day's value is shown properly. The 'total' is actually calculated as (last day downloads - first day downloads), so it's not simply a sum of daily downloads. This is probably done to account for days for which data is missing.
lionscribe
commented
11 years ago I am using 2.6.0 The value for each days "Total Downloads" is correct. The value for the first day's "Downloads by Day" shows 0. Also the calculation "last day downloads - first day downloads" is incorrect. It should be "last day downloads - day before first downloads" (You don't want 7 minus 1, you want 7 minus 0).
nelenkov
commented
11 years ago I am not seeing those 0's so this might triggered by your current data, etc. Can you try to debug it?
lionscribe
commented
11 years ago I looked at the code (getStatsForApp() in ContentAdapter.java), and the problem is simple. When we set for example TimeFrame to Timeframe.LAST_SEVEN_DAYS, we set the limit to 7. Therefore we are only querying the data for seven days, which does not give us the data for the "day before 1". Therefore we cannot calculate daily downloads for first day, nor for the whole week. We are always missing the first days downloads!
The code should be changed to set the limit to 1 more than limit, for example for week, set it to 8. Then all calculations will be correct. After all claculations call
if (!currentTimeFrame.equals(Timeframe.UNLIMITED)) {
limit--;
}
before calling
if (result.size() > limit) {
result = result.subList(result.size() - limit, result.size());
}
to get rid of the extra first day that was needed for calculations.
I did not check all the code, but this should be it basically.
nelenkov
commented
11 years ago Getting data for one more day obviously works, but summary code (AppStatsSummary.calculateOverallStats()) tries to do a bit more (like interpolating missing values, which may not be correct in call cases), so this doesn't quite work. If you can post an actual working patch for this, that would be appreciated. The calculateOverallStats() code comes from the original app, so pretty much no one knows exactly why it works the way it does :)
lionscribe
commented
11 years ago I will look into it. I didn't realize that the dev branch is so much different than the master branch. The master branch doesn't even have a AppStatsSummary class.
lionscribe
commented
11 years ago Giving a quick glance, I cannot see any problem with adding the day.
All we need to change is 2 lines of code in getStatsForApp
int limit = Integer.MAX_VALUE - 1; // We have to deduct 1 as we add 1 later on in the query
and
AppStatsTable.KEY_STATS_REQUESTDATE + " desc LIMIT " + (limit+1) + "");
The extra day will get removed after the computations in calculateOverallStats by the current code
if (stats.size() > limit) {
stats = stats.subList(stats.size() - limit, stats.size());
}
This all happens before the ratings are calculated, so I don't see an issue with that.
When viewing the download report, the first day's downloads are shown as 0. For example, when you filter it to show downloads for last 7 days, the first day's downloads is shown as 0, and the total is only for the last 6 days. Same is with the 30 day report. This makes sense when viewing all downloads (unlimited), for then it does not have a previous day's totals in order to calculate the first days downloads, hence it's set to 0, but when filtering to last 7 or 30 days, it really does know the previous day's downloads, and it should show even for first day.