nateP82 / compostCollectors

Repo for the compost collection site
0 stars 0 forks source link

Peer review #1 - Abigail Genz #2

Open amgenz15 opened 1 year ago

amgenz15 commented 1 year ago

Project: Compost Collectors

Developer: Nate Peck

Reviewer: Abigail Genz


@nateP82 Below is my peer review for you! I am loving your project so far and I can't wait to see the outcome!
pawaitemadisoncollege commented 1 year ago

Hi @amgenz15 Thanks for your thorough review and keen eye on the details.

  • [ ] I did find a couple sneaky printStacktraces - since these are in the interface, it may not be possible to use a logger, so the exceptions may need to be thrown up to the caller to be handled. stacktrace
ItemConsiderationsComments/Suggestions
Reviewer comments and suggestions go here. Each item should have at least one "kudos" and two suggestions for improvement
Problem Statement1. Accurately describes project purpose
2. Is professional and free of typos, slang, etc.
3. Fully explains the problem and the solution
4. Is understandable by the average person
The problem statement for Compost Collectors accurately describes the project details and what a register user will have access to, it gives a full explanation of the problems composers find themselves in ex: tools, space, time. It is also written so anybody can understand what problem the project is trying to solve. A few nit picky items to touch up on is a few grammar and spelling errors. 1. "You may not know what can and what cannot be composted. " The second what can be removed for more clarity. 2. "Compost collectors looks to help users" - "looks" should be "look" 3. Lastly there are just two spelling errors that I caught: "scheduled" which is in this line "View sheduled pickup times" and "several" which is in this line "advantage of serveral features"
Design Documentation1. 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
I really like the detail you put forth in your screen designs. From what I can tell all of the user stories and screens align nicely! In the meeting we had you showed me the flow of the screens in figma and I thought they were laid out in a very logical order. I think to allow all users that are viewing your project design you should create an .md file that shows the flow of the screens similarly to how you showed me in figma.
Data model/Database1. Everything on the screens and problem statement/flow is represented in the model
2. There is at least one 1-to-many relationship.
3. The model represents good database design
Everything is well represented in the database model and data! You have a 1-many relationship between the user and the pick-up service and the pick-up service is appropriately tied to the user table. While I was taking a closer look at your project I had noticed that there wasn't a place where you were storing a users contributions. I was wondering how you were going to store that data? Maybe make a new table for each contribution that a user makes so it will be easier to track it for the graph so you can store the date and time that the contribution was made and how much was contributed?
Code1. 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.
6. There are appropriate unit tests/code coverage.
In your project you have successfully implemented the maven project structure. You don't have any monster classes and everything has a specific purpose. I also like how you used logging statements in your JUnit code to show which test is doing what. I also like your test code coverage. One thing I noticed while going back to review your project was making sure your tests are using the overridden equals method. I saw only one missed instance in the User entity in the updateSuccess() test.