Brad-Davidson / MealPlanner

0 stars 1 forks source link

Code review changes #23

Open IsaiahDicristoforo opened 3 years ago

IsaiahDicristoforo commented 3 years ago

Analysis of the program

Your program is off to a great start! The user interface is simple and polished. The autocomplete text box works well, and your UI logic is decoupled from the DTO logic. Obviously, there are still some features that have to be implemented, and DTO classes that need to be integrated into the UI components, but you have a foundation for adding this functionality in.

Was the program available in UC Github on time? Yes, I had no issues getting access to the program in order to complete my code review.

Is the program documented/commented well enough for you to understand? The program was very well organized, so it wasn't difficult to find certain features. The program would benefit from the addition of some comments and Kdocs above some of the methods but at this stage, it is easy to figure out what methods do.

Does the program compile? Yes, the app launched with no issues.

Rationale behind your changes. When I first launched the app, it was on a slightly smaller phone, so the buttons at the top of the UI ran off the screen. I made some UI changes to fix this, and ensure the top buttons stay centered and on screen. I did this by putting the buttons in a horizontal layout and adjusting the constraints. The other changes are very minor. I changed the format of some of the variable names just to follow Kotlin conventions. Finally, you had a companion object which was unnecessary. Your companion object created a new instance of the MainFragment class. I streamlined this process by removing the companion object and just creating a MainFragment object directly.

Three Technical Concepts I learned

I got a better understanding of how to work with constraint layouts and how to position and center items inside of them. I got a better grasp of how to use annotations such as @Query, @Insert, and @PrimaryKey I learned how to integrate and use room db in a project. My team is using firebase, so it was interesting to compare and contrast the two databases.

discospiff commented 3 years ago

Several good suggestions here that align with best practices and reduce technical debt.

The team should discuss and consider merging.