Conversation
…restructured class to fit Kotlin style guides
| var movieCountry : String = "", | ||
| var movieBoxOfficeGross: String = "", | ||
| var movieOpeningWeekendGross : String = "", | ||
| var movieDistributor : String = "") { |
There was a problem hiding this comment.
Made first letter of variable names lowercase based on Kotlin naming conventions (https://kotlinlang.org/docs/coding-conventions.html#naming-rules)
| override fun toString(): String { | ||
| return "$movieTitle" | ||
| return movieTitle | ||
| } |
There was a problem hiding this comment.
Removed redundant string formatting
| btn_FindMyMovie.setOnClickListener { | ||
| val btnFindMyMovie = findViewById(R.id.btn_FindMyMovie) as Button | ||
| btnFindMyMovie.setOnClickListener { | ||
| // your code to perform when the user clicks on the button |
There was a problem hiding this comment.
Removed underscore from variable name based on Kotlin naming conventions (https://kotlinlang.org/docs/coding-conventions.html#naming-rules)
There was a problem hiding this comment.
I don't see this change...
We use underscores for constants. Otherwise, correct, we typically don't use them in variable names.
|
|
||
| val btnFindMyMovie = findViewById(R.id.btn_FindMyMovie) as Button | ||
| val btnFindMyMovie = findViewById<Button>(R.id.btn_FindMyMovie) | ||
| btnFindMyMovie.setOnClickListener { |
There was a problem hiding this comment.
Android Studio recommended formatting
|
|
||
| class Specimen(var movieTitle: String = "", | ||
| data class Specimen(var movieTitle: String = "", | ||
| var movieRank: Int = 0, |
There was a problem hiding this comment.
Declared 'Specimen' as a data class to tell compiler class is for holding data
| import com.google.gson.reflect.TypeToken | ||
|
|
||
| public data class Movie(@SerializedName("rank")var rank: Int, @SerializedName("title")var title: String, | ||
| data class Movie(@SerializedName("rank")var rank: Int, @SerializedName("title")var title: String, |
There was a problem hiding this comment.
Removed redundant visibility modifier
|
|
||
| implementation 'com.squareup.retrofit2:retrofit:2.8.2' | ||
| implementation 'com.sqeareup.retrofit2:converter-gson:2.7.1' | ||
| implementation 'com.squareup.retrofit2:converter-gson:2.7.1' |
|
Several good suggestions here, which reduce technical debt. I recommend merging this branch. |
2 participants
Analysis of the program
(Note: My code review is based on the branch with most and recent commits, ColinMaxwellSprint3Branch_4)
Overall, there’s some good progress being made on this app!
Was the program available in UC Github on time?
Yes
Is the program documented/commented well enough for you to understand?
Does the program compile?
There was some red lined code upon opening the project, but this isn’t the main branch so it’s most likely a work in progress
Rationale behind my changes
All of these changes aim to reduce technical debt of the in-progress project and allow it to be more readable as well as allow more functionalities to work
Three technical concepts I learned
Three of my commits