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

Jwen/end game download #345

Merged
merged 23 commits into from
May 26, 2022
Merged

Jwen/end game download #345

merged 23 commits into from
May 26, 2022

Conversation

jiabaow
Copy link
Collaborator

@jiabaow jiabaow commented May 24, 2022

No description provided.

*/
fun downloadListener(songView: TextView?, songName0: String?) {
if (downloadStarted) {
Toast.makeText(applicationContext, getString(R.string.download_already_downloading), Toast.LENGTH_LONG).show()
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 3 locations. Consider refactoring.

if (!properties.exists()) {
properties.createNewFile()
}
Helper().setReturnToMainListener(findViewById<FloatingActionButton>(R.id.download_returnToMain), this)
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 3 locations. Consider refactoring.

findViewById<FloatingActionButton>(R.id.delete_returnToMain).setOnClickListener {
IntentSwitcher.switch(this, MainActivity::class.java)
}
Helper().setReturnToMainListener(findViewById<FloatingActionButton>(R.id.delete_returnToMain), this)
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 3 locations. Consider refactoring.

findViewById<FloatingActionButton>(R.id.end_returnToMain).setOnClickListener {
IntentSwitcher.switch(this, MainActivity::class.java)
}
Helper().setReturnToMainListener(findViewById<FloatingActionButton>(R.id.end_returnToMain), this)
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 3 locations. Consider refactoring.

val broadcast = object: BroadcastReceiver() {
override fun onReceive(context: Context?, intent: Intent?) {
val id = intent?.getLongExtra(DownloadManager.EXTRA_DOWNLOAD_ID, -1)
if (id == downloadId) {
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.

@Tsathogguaa Tsathogguaa marked this pull request as ready for review May 26, 2022 15:20
var downloadStarted = false
}

private val STORAGE_PERMISSION_CODE = 1000
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good use of constants

@MaximeZmt MaximeZmt self-requested a review May 26, 2022 15:24
@@ -140,4 +142,32 @@ class GameEndingActivity : AppCompatActivity() {
}
}
}

override fun onItemClick(position: Int) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe use some empty lines to separate logic and improve readability

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will be on the next commit!

Copy link
Collaborator

@laurislopata laurislopata left a comment

Choose a reason for hiding this comment

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

Looks good to me, just added a small comment, if you want you can fix it another time

@Tsathogguaa
Copy link
Collaborator

Looks good to me, just added a small comment, if you want you can fix it another time

Thanks! Noticed I forgot to add documentation for some of the methods which I will be doing now!

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, small details below

Comment on lines 44 to 45

fun createDownloadReceiver(songNameView: TextView?) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
fun createDownloadReceiver(songNameView: TextView?) {
/**
Missing doc for public function
*/
fun createDownloadReceiver(songNameView: TextView?) {

Comment on lines 22 to 23

open class DownloadFunctionalityActivity : AppCompatActivity() {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
open class DownloadFunctionalityActivity : AppCompatActivity() {
/**
Missing doc for class
*/
open class DownloadFunctionalityActivity : AppCompatActivity() {

Copy link
Owner

Choose a reason for hiding this comment

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

I think you forgot that one 😅

songName = songName0
}

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.

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.

missing one

Comment on lines 22 to 23

open class DownloadFunctionalityActivity : AppCompatActivity() {
Copy link
Owner

Choose a reason for hiding this comment

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

I think you forgot that one 😅

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.

LGTM 👍 😃

Copy link
Collaborator

@laurislopata laurislopata left a comment

Choose a reason for hiding this comment

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

LGTM!

@codeclimate
Copy link

codeclimate bot commented May 26, 2022

Code Climate has analyzed commit 2cb3206 and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 6

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

This pull request will bring the total coverage in the repository to 85.7%.

View more on Code Climate.

@Tsathogguaa Tsathogguaa merged commit 0312fdc into main May 26, 2022
@Tsathogguaa Tsathogguaa deleted the jwen/end-game-download branch May 26, 2022 16:57
@jiabaow jiabaow linked an issue May 27, 2022 that may be closed by this pull request
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.

Add downloading ability to the buttons that are in the end game screen
4 participants