Code Review 1 #10
Code Review 1 #10
Conversation
| universities.postValue(innerUniversity) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Added MainViewModel as it didn't had it.
There was a problem hiding this comment.
This looks like an incomplete feature.
Are you using these anywhere? I don't see them used.
If not in use, remove them. Unused code is clutter, and bad documentation, which increases technical debt.
Only complete features, which meet the team's Definition of Done, should be merged to Main/Master.
| suspend fun fetchUniversities(): List<University>? | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
Delete the IUniversityService and ddd this in the in the UniversityService. You can just code this here.
| @@ -11,5 +11,5 @@ interface IUniversityDAO { | |||
| * @return List of universities | |||
| */ | |||
| @GET("/search") | |||
| fun getAllUniversities() : Call<ArrayList<University>> | |||
| fun getAllUniversities() : Call<List<University>> | |||
There was a problem hiding this comment.
Since for other you been using List so just use List instead of arrayList
There was a problem hiding this comment.
Good. This is the "D" in SOLID: Dependency Inversion. Rely on the interface, not the concrete implementation.
|
From this review: Consider the dependency inversion piece: classes should refer to interface types, not other classes. Other additions are likely needed for the project at some point, but are new, and sometimes incomplete, functionality. New functionality is better in a separate branch until fully developed, as features should not be merged to main/master until the meet the team's Definition of Done. Code review branches should minimize technical debt, and thus, be merge-able to master immediately. So, if the team desires these features, I'd keep the branch open, and iterate over the feature until it is complete. |
Analysis of the program.
They have the foundation of the project(dao,dto,service)
Was the program available in UC Github on time? Yes
Is the program documented/commented well enough for you to understand? Yes
Does the program compile? Yes
Rationale behind your changes.
Was kind of hard as they didn't had much to work on. But found something to put and change to make the 3 changes
Links to three specific commits that you made to your own group's GitHub repository before the end of sprint deadline.
If you do not post links to three commits that you made to your group's GitHub repo, your code review grade will be subject to a 20% penalty.
1: Jovial-Developer/IT3048CSpringProject@296fe3a
2: Jovial-Developer/IT3048CSpringProject@ce0c633
3: Jovial-Developer/IT3048CSpringProject@2ecf444