Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Further changes to downloading structure and removal of downloaded files #205

Merged
merged 9 commits into from
Apr 28, 2022

Conversation

Tsathogguaa
Copy link
Collaborator

image
image
image
image
image

The implementations for a downloading system with duplication checks and systematic recording of currently downloaded files have been completed. Also introduced a removal activity, which lists the various downloaded files that are tracked in the said recording for the user to delete.

layout.addView(noDownloadsText)
}

fun switchToWelcome(view: View) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 4 locations. Consider refactoring.

if(!records.exists()) {
records.createNewFile()
}
records.appendText("$songName\n")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 4 locations. Consider refactoring.

* If the file does not exist, it is created.
*/
private fun record() {
var records = File(applicationContext.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS), "records.txt")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 4 locations. Consider refactoring.

}
}

private fun deleteDownloadedSong(btn: View, layout: LinearLayout): Boolean {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method deleteDownloadedSong has 34 lines of code (exceeds 25 allowed). Consider refactoring.

generateButtons(layout)
}

private fun generateButtons(layout: LinearLayout) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method generateButtons has 27 lines of code (exceeds 25 allowed). Consider refactoring.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could refactor the while loop for example in an inner function to improve maintainability?

}

private fun sendDirectIntent(arg: Class<*>?) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical blocks of code found in 4 locations. Consider refactoring.

sendDirectIntent(DeleteSongsActivity::class.java)
}

private fun sendDirectIntent(arg: Class<*>?) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical blocks of code found in 4 locations. Consider refactoring.

sendDirectIntent(WelcomeActivity::class.java)
}

private fun sendDirectIntent(arg: Class<*>?) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical blocks of code found in 4 locations. Consider refactoring.

return deleteButton
}

/*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method deleteDownloadedSong has 34 lines of code (exceeds 25 allowed). Consider refactoring.

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job overall 👍 . Just left some minor changes and questions below.

@MaximeZmt
Copy link
Owner

Nice job overall 👍 . Just left some minor changes and questions below.

In the code below There is the same problem in the doc as mentioned in other part but as it is old code, I cannot comment it from download Activity.

/*
* Sets the hint of the given textview with the given hint, and clears the entered text.
*
* @param hint : String to be set as the hint of the textView.
* @param songNameView : The textView that will be updated.
*/
private fun editTextView(hint: String, songNameView: TextView) {
songNameView.text = ""
songNameView.hint = hint
}

/*
 * Checks if the required app permissions are already given. If not, request those permissions.
 */
private fun checkPermissionsAndDownload() {

return deleteButton
}

/**
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method deleteDownloadedSong has 34 lines of code (exceeds 25 allowed). Consider refactoring.

downloadComplete = false
songName = songView.text.toString()

if(checkExistingSong()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Apr 28, 2022

Code Climate has analyzed commit 65ed34a and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 2

The test coverage on the diff in this pull request is 84.1% (80% is the threshold).

This pull request will bring the total coverage in the repository to 89.9% (0.8% change).

View more on Code Climate.

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work 👍

Copy link
Collaborator

@zwierski zwierski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. There could be some (non-urgent) refactoring to do to make some functions shorter by adding calls to smaller functions instead.

@Tsathogguaa Tsathogguaa merged commit 8dc786b into main Apr 28, 2022
@Tsathogguaa Tsathogguaa deleted the tsathogguaa/further_download branch April 28, 2022 19:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants