Kharsla / GroceryListGenerator

1 stars 0 forks source link

Design Review #5

Open akcuster opened 3 years ago

akcuster commented 3 years ago

Design/Code Review 1

Project: GroceryListGenerator

Developer: Kaitlyn Harsla

Reviewer: Amber Custer

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.
The project's purpose was verbally explained very clearly. I was unable to find the problem statement in the repository though, so I can't really comment on the rest of it. A README.md file with the problem statement and links to the design documents could be useful
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 wireframes are well done and the flow made sense when you walked through them. So far, only the sign up is demonstrated in the ApplicationFlow file. It looks good overall, but is a little confusing as to whether it's for sign up or sign in or both? Some more clarification on that would be good. The user stories are clear and make sense. Formatting the file would help readability.
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.
No data model or database design document to comment on
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 project structure looks good. There's the standard .gitignore file. The classes that are done all look great. Logging hasn't been implemented yet, but there were no System.out.println. The tests for the available Daos look good. I noticed a few missing JavaDoc comments in the Daos and tests.
pawaitemadisoncollege commented 3 years ago

@Kharsla and @akcuster One thing to add to this, there are some sneaky System.out.printlns and stack traces out there. Don't you love it when instructor-provided code has built-in standards "violations"?

Screen Shot 2021-02-26 at 2 15 08 PM