noahaguilera / VideoGameCollectionTracker

Keep track of your games across all gaming platforms
0 stars 0 forks source link

Week 6 Code Review #3

Open ClassyElm opened 3 years ago

ClassyElm commented 3 years ago

Design/Code Review 1

Project: Video Game Collection Tracker

Developer: Noah G.

Reviewer: Peter C.

Item Considerations Comments/Suggestions
Reviewer comments and suggestions go here. EVERY item should have at least one "kudos" and one suggestion for improvement
Problem Statement 1. Accurately describes the project's purpose.
2. Is professional and free of typos, slang, etc.
3. Thoroughly explains the problem and the solution.
4. Is understandable by the average person.
Despite some informal wording, the project statement was very clear about the problem and solution users are faced with as a result of the ever-growing selection of game platforms and video games. I especially appreciated the inclusion of physical game copies because, though it is increasingly less common, there is still a market for them. My biggest critique of the problem statement is the lack of references to similar services already existing.
Design Documentation 1. Navigation/flow through the application is logical and easy to use.
2. The order in which values are displayed are logical and easy to understand/use
3. The order in which the form fields entered are logical and easy to understand/use
4. All data discussed/documented (problem statement, flow, db design, etc.) is represented on the screens.
5. Project plan and time log are up-to-date.
The application flow appears to be completely visualized in a logical and easy-to-use manner. One aspect of the design documentation that particularly stands out to me is how well thought out the application design pages are. Overall, the only thing I would add to the design documentation is a screen for when users attempt to reset their password.
Data model/Database 1. Everything on the screens and problem statement/flow is represented in the model.
2. There are at least two 1-to-many relationships.
3. The model represents good database design.
Everything in the screen design and problem statement acccurately describe the relationship between users and their games. I felt it was impressive that a working, well-formed database is already in use and working with JSPs. One suggestion I have is to remove the column annotations from instance variables that match the same name as the columns in the table. It is not necessary to have them when they match (though I guess it is more clear how the table is laid out).
Code 1. Proper Maven project structure is used.
2. a .gitignore file for IntelliJ Java projects has been implemented.
3. There is not any redundant or copy/paste code in the JSPs or classes.
4. Classes are appropriately-sized (no monster classes).
Property files are used appropriately: no hard-coded values.
5. Logging statements are used rather than System.out.println and printStackTrace (be sure to use a search to look for these!).
6. There are appropriate unit tests/code coverage.
The source code of your project looks very clean and well-documented! I found it really easy to go into any given source code file and understand the inner-workings. My only critique here is to utilize a generic DAO in place of your UserDao, GameListDao, and UserGameListDao. This will eliminate duplicate code in the DAOs. Of course, be sure to udpate the unit testing if you do change over to a generic DAO.
pawaitemadisoncollege commented 3 years ago

@ClassyElm and @noahaguilera

Be sure to clean up the sneaky printlns and stack traces for checkpoint 2:

Screen Shot 2021-02-26 at 2 25 32 PM