Offline mode implementation #277
Conversation
Quick note about why the code in DownloadManager is commented and not tested: The tests would require both a download and an API call, both of them are not stable things to test. If the LyricsBelongGameActivity tests can be injected somehow, then I can suggest testing this piece of code as well, otherwise the tests may fail very often, both because of the API and the download activity. Since our coverage is low and I currently do not need this piece of code(not until next week's PR), I left the code commented to not decrease the coverage for no use. |
| var media = File(context.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS), "extract_of_$currentSong.getTrackName()") | ||
| mediaPlayer = AudioPlayer.playAudio(media.absolutePath) | ||
| */ | ||
| } |
There was a problem hiding this comment.
Could you just put these modifications into GameManager instead? I deleted BuzzerGameManager when refactoring and put all the equivalent functions into GameManager instead so you should find it there
There was a problem hiding this comment.
Done on local, will push soon once I make sure the merge was successful
| page = "1" | ||
| songsPerPage = "100" | ||
| totalPages = "20" | ||
| totalSongs = "2000" |
There was a problem hiding this comment.
These vars don't seem to be modified anywhere in the class: maybe it'd be more readable to turn them into private vals / constants that you directly initialise as they're declared.
There was a problem hiding this comment.
True, will do it
| var records = File(context.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS), "records.txt") | ||
|
|
||
|
|
||
| if(!records.exists() || records.length() == 0L) { |
There was a problem hiding this comment.
Add a space after if for readability if (!records.exists() || records.length() == 0L)
There was a problem hiding this comment.
Done on local
| var reader = BufferedReader(FileReader(records)) | ||
| var currentLine = reader.readLine() | ||
|
|
||
| while(currentLine != null) { |
|
|
||
| while(currentLine != null) { | ||
| var trimmed = currentLine.trim() | ||
| if(trimmed.isNotEmpty()) { |
| var trimmed = currentLine.trim() | ||
| if(trimmed.isNotEmpty()) { | ||
| val split = trimmed.split("-") | ||
| if(split.size == 2) { |
| private fun recordProperties() { | ||
| var properties = File(applicationContext.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS), "properties.txt") | ||
|
|
||
| if(!properties.exists()) { |
Went over them and committed the changes. Can you quickly check if they are better now? |
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| return songList | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
| val totalPages = "20" | ||
| val totalSongs = "2000" | ||
|
|
||
|
|
|
|
||
| init { | ||
| try { | ||
|
|
|
Code Climate has analyzed commit 8b297d0 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 83.3% (80% is the threshold). This pull request will bring the total coverage in the repository to 80.3% (0.2% change). View more on Code Climate. |
Making necessary preparation to implement the offline usage of the app. Mostly done. Some of the code that is added is in the form of commentaries, as the required parts to compile those lines have yet to be implemented.
Have to discuss test conditions with the team.