Code Review Ready!

[redfoxanna]

Hi @MNuenninghoff @pawaitemadisoncollege! I have completed the code review for you!

Design/Code Review 2

Project: NPC Generator

Developer: Michael Nuenninghoff

Reviewer: Anna Kessler

Areas for Improvement	Criteria	Items Met or Exceeded
Project effectively utililizes the technologies and technques specified in the project objectives	Uses all technologies and techniques described in the README.md
Planned MVP Functionality	Planned MVP is met
Logging framework used, i.e., no System.out.println() or printStacktrace() statements.	No system.out.println or printStackTrace() found in code
Hibernate used for all data access.	All entities are mapped to use hibernate and a GenericDao for data access
Authentication implemented and adds value to the application.	Cognito implemented and I was able to sign up as a new user, verified my email and then logged in successfully
Consumes at least one web service or public api using Java.	Uses muna.ironarachne.com name generator
Application is database-driven using full CRUD.	A user is able to create an NPC and then update, delete or read the NPC from the database
Database includes multiple one to many relationships.	There are at least 10 one to many relationships to one User
Deployed to AWS for public access.	Yes, but might need to upload your latest war file to EB
Could use a properties loader to load all of your applications properties at startup	Implements best practices (for example, data validation)
Synthesis of multiple concepts in unfamiliar situations requiring research beyond the scope of the class
Experiments individually, exhibits independence and drive, shows originality in the solution.	Fun idea for an application. I've seen a DM have to make up a character mid-game and this could save game play time
Implemented technologies or techniques not covered in course materials.	Random name generator
Code quality - Evaluate code quality for the following and identify specific areas for improvement (class, method or line number). Be sure to list which code quality plugins/tools you used to assist with this analysis
Single-purpose methods	Yes, single purpose methods used
The rerollAttribute method could be refactored to improve readability. Consider breaking it down into smaller methods, each responsible for rerolling a specific attribute.	Well-structured project
Descriptive naming of packages, classes, methods, variables	I am able to understand the purpose of packages, classes, etc by naming used
Classes appropriately-sized (no monster classes)	Nothing seems too large but your largest class could be smaller with refactoring mentioned two comments above this
CPD (copy paste detection, meaning are the same lines of code repeated?	No repeated lines of code
Are there candidates for super/subclass relationships, abstract classes, interfaces	No obvious candidates
are any values hard-coded that should be in a properties file?	None found
Proper exception handling	Exception handling is proper
Proper error reporting to the user - custom error pages?	Custom error page implemented
In GenerateActionServlet the javadoc comments could be added	Code documentation	appropriate javadoc used throughout application
Breaking each action: generateNPC, saveNPC, deleteNPC, etc. into separate methods would make the coe easier to test	Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods?
Adding some styling to your jsps could really add a lot to your application	Evaluate the JSPs for templating, data validation, overall look and feel.	jsp templates are used for: head, index, navbar, view and edit. Do you want one for delete also?
JSPs use JSTL and EL, no java code	No java code found in jsps
Many of your unit tests don't have search by property equal methods	Unit tests are truly a unit test rather than a high level functional test	Each of your entities have a unit test
Test data is appropriately cleaned up or handled	Uses cleanDB.sql for data clean up
Test to ensure that the generateNewNPC method returns a non-null NPC object when provided with an HttpSession object	There is full coverage of methods that perform business logic or utility functions
Redundant code is eliminated by using set up and tear down methods, i.e., @Before, @After	Used @BeforeEach to clean out db before each unit test
Other comments/notes?
It doesn't seem like you have updated your project plan for a few weeks	Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) to meet milestones and project objectives.
Evidence of significant revision and incorporation of feedback.	Looking over checkpoints, professor feedback and commits you have been making revisions and incorporating feedback
What does github history and insights demonstrate about the work on this project?	Looking at the commits, there are many over the semester and pretty consistent
Project complexity
Consider the user display exercise to be a simple project; how does this project compare?	Slightly more complex than the user dispay exercise
What aspects of the project add complexity?	The amount of one to many relationships, authentication
The description of an NPC doesn't persist on your edit page	Additional Comments

[pawaitemadisoncollege]

Hi @redfoxanna - thanks for providing this review of @MNuenninghoff's project. Your feedback in each column provides specific detail, and the areas for improvement are all actionable - meaning you provided enough information for the reader to understand what needs to be done/could be done. I forgot to ask this in Michael's review of your work, so to both of you - did you use any tools to help identify best practices related to code quality? If so, it can be helpful to attach that report here and summarize any "easy wins" it identifies.