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

Maximezmt/typing game #81

Merged
merged 45 commits into from
Mar 17, 2022
Merged

Maximezmt/typing game #81

merged 45 commits into from
Mar 17, 2022

Conversation

MaximeZmt
Copy link
Owner

@MaximeZmt MaximeZmt commented Mar 12, 2022

Typing Game

This is related to issue #64

This Pull Requests contains:

ch.sdp.vibester.api: BitmapGetterApi: an api with a function download(...) that will return a CompletableFuture of a Bitmap
ch.sdp.vibester.games: TypingGame: An activity that represent a game where the user has to type to guess a music

In addition the code is tested with a coverage of: [89]%

images1Pres
images2Pres

Notes for reviewers

  • In the build.gradle file for the app, I've updated some part of it to force using JaCoCo version: 0.8.7. Otherwise I causes problem with code coverage detection Related bug found on internet
  • Code reviewing will be done in the next sprint

@MaximeZmt MaximeZmt self-assigned this Mar 12, 2022
@MaximeZmt MaximeZmt linked an issue Mar 15, 2022 that may be closed by this pull request
@MaximeZmt MaximeZmt marked this pull request as ready for review March 16, 2022 17:59

}

override fun onCreate(savedInstanceState: Bundle?) {
Copy link

Choose a reason for hiding this comment

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

Method onCreate has 59 lines of code (exceeds 25 allowed). Consider refactoring.

return newIntent
}

fun guess(song: Song, guessLayout: LinearLayout, ctx: Context, playedSong: Song, player: CompletableFuture<MediaPlayer>?): FrameLayout{
Copy link

Choose a reason for hiding this comment

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

Method guess has 5 arguments (exceeds 4 allowed). Consider refactoring.


}

override fun onCreate(savedInstanceState: Bundle?) {
Copy link

Choose a reason for hiding this comment

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

Method onCreate has a Cognitive Complexity of 49 (exceeds 20 allowed). Consider refactoring.

}
val list = Song.listSong(task.await())
for(x: Song in list){
if (mysong != null) {
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.

for(x: Song in list){
if (mysong != null) {
guess(x, findViewById<LinearLayout>(R.id.displayGuess), this@TypingGame, mysong, mediaPlayer)
}else{
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
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.

As you said some refactoring is needed but we can focus on that next week, other than that LGTM

@codeclimate
Copy link

codeclimate bot commented Mar 17, 2022

Code Climate has analyzed commit e5b6945 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3
Duplication 2

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

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

View more on Code Climate.

Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

👍

@@ -25,8 +25,8 @@ class ItunesMusicApi private constructor(){
* @param baseUrl (Optional) If you want to specify an other url to query
* @return CompletableFuture<String> that contains the result of the query
*/
fun querySong(query: String, okHttp: OkHttpClient, baseUrl: String = LOOKUP_URL_BASE): CompletableFuture<String> {
val buildedUrl = baseUrl+query.replace(' ', '+')
fun querySong(query: String, okHttp: OkHttpClient, limit: Int, baseUrl: String = LOOKUP_URL_BASE): CompletableFuture<String> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget to update the documentation for limit

@MaximeZmt MaximeZmt merged commit 40dce50 into main Mar 17, 2022
@MaximeZmt MaximeZmt deleted the maximezmt/typingGame branch March 17, 2022 12: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.

Doing a feature to query the music by name (#54)
3 participants