Peer Code Review

[jordynbx]

Design/Code Review 1

Project:

Developer: Heather Kurth

Reviewer: Jordyn Barbiaux

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.	Your problem statement accurately describes the projects purpose, explains the project and solution, and is very understandable! Overall I think it looks great. My only suggestion here would be to clean up what questions/notes you left for yourself as you become more confident in what will be part of V1 of your project and what is part of a potential future release.
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.	Your design documentation looks great. I love the use of color in your wireframe/screen representations, they make it easy to see exactly how the app will function and integrate well with your Application Flow. Your user stories do a great job of showing what the purpose of this application is, how it will benefit members of the community, and how different community members will utilize the functionality. There appears to be some boilerplate text referencing trails left over in the Application Flow document, so I would take a look at that! I would also recommend adding some sub-folders inside your Design Documents folder to make it easier for others reviewing your application to navigate. You also have an extra document in there, wireFrames.png that is a database map of the Trails application. Your time log/reflections look really thorough and well-documented but are a bit difficult to read - I wonder if it's possible to just upload a word document to Github as as opposed to doing screenshots.
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.	You covered a lot of info in your database! You have at least two one to many relationships and everything covered in your problem statements is present. I like how you split out the different info for your tables. I used to work in a non-profit and we worked with organizations who had overhead organizations, multiple locations, specific contacts, and different owners who didn't want to be bothered. To me your design accurately reflects the structure of this world and shows you're very familiar with the industry and how it works! Your database design is a good start, but there are a few changes you might want to consider: Is the website column in food_resources and resource_owners referring to the same website? If so, you might not need it in both columns. Some of your columns might be bigger than necessary for the data you're storing which can slow down your application (for example - service_area at 1000) There is a description field in resource_types and food_resources, but if resource_types is meant to categorize different types of resources you likely don't need the description column there because it will be covered in the individual resource descriptions. Another option would be to shorten it to 50-100 character with a brief description of that type of resource, if that data will be present somewhere in your app. It looks like you have an id column for food_resources in the resource_types tables. I'd actually recommend doing the opposite, since you want to have each food resource categorized by a specific type from the resource_types table. The way it's laid out right now you will only be able to identify one food resource of each type, when I think your intention was to have all resources be one of the types listed. You do have a column for resource types in the food_resources table but it's not linked to the resource_types table. I'd recommend removing the resource_type column from the food_resources table and replace it with a resource_types_id foreign key column that corresponds to one of the types in the resource_types table, and then removing the food_resource_id column from resource_types so that multiple food resources can be associated with a type. * In the resource_owners table you have types_offered which is plural. Assuming a resource owner will offer more than one type of resource, you might consider breaking that out into a new table where resource_owners are mapped to resource_types by id of each (i think this would be many to many) so that you don't have duplicative info in the resource_owners table if a resource owner offers multiple types (this is only relevant if a resource owner will provide multiple types).
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 proper Maven structure is used, but I'm not able to find a .gitignore file. Make sure you add one in, unless I just missed it somewhere. Maven structure looks good and classes are appropriately sized. I love that you used a superclass for organization and thinking of the future. That's a great way to plan ahead and definitely something I will consider in my own project. Unit tests look good. Logging statements look good and I don't see any system.out.printlns. Based on screenshots test coverage looks good. One small thing I noticed - in the ResourceOwner class you have a set of FoodResourceTypes, but you don't have a class named FoodResourceTypes. It does correspond to your DB though so you might just not have created it yet.

[pawaitemadisoncollege]

@jordynbx and @hmkurth One thing to add to this, there are some sneaky System.out.printlns/stacktraces out there. Don't you love it when instructor-provided code has built-in standards "violations"? :(

[hmkurth]

Thanks so much @jordynbx, this is great feedback and i have already started implementing some of the changes you suggested!