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

Small refactoring and some documentation #240

Merged

Conversation

Tsathogguaa
Copy link
Collaborator

Just took care of some maintainability issues in some files, and added a bit of documentation to some of the files that are missing them.

@Tsathogguaa Tsathogguaa self-assigned this May 5, 2022
}
return finalVal
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good to have refactored that into a separate function, it avoids duplications and lengthy functions. 👍

@@ -51,27 +53,18 @@ class GameEndingActivity : AppCompatActivity() {
} else {findViewById<TextView>(R.id.winnerText).text="Nobody won this game!"}
}

getFromIntent(intent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, much more readable than adding all the if's directly into the function.

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, good job especially on documenting all the functions!

@@ -35,8 +38,12 @@ class LyricsBelongGameActivity : GameActivity() {
val getIntent = intent.extras
if (getIntent != null) {
gameManager = getIntent.getSerializable("gameManager") as GameManager
setNextButtonListener(ctx, gameManager)
setCheckButtonListener(ctx)
findViewById<Button>(R.id.nextSongButton).setOnClickListener {
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 May 5, 2022

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

Here's the issue category breakdown:

Category Count
Duplication 2

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

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

View more on Code Climate.

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.

Good work refactoring some code. LGTM!

@Tsathogguaa Tsathogguaa merged commit e51769a into main May 5, 2022
@Tsathogguaa Tsathogguaa deleted the tsathogguaa/documentation_and_possible_maintainability branch May 5, 2022 16:35
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