Peer Code Review 2

[MNuenninghoff]

Design/Code Review 2

Project: Diamond Poems

Developer: Anna Kessler

Reviewer: Michael Nuenninghoff

Areas for Improvement	Criteria	Items Met or Exceeded
Project effectively utililizes the technologies and technques specified in the project objectives
In progress on completing aspects of the minimum viable product (sign in, edit, seeing which user uploaded a poem)	Planned MVP Functionality	Can view past poems and search poems
Logging framework used, i.e., no System.out.println() or printStacktrace() statements.	No print statements found. Log4J used for logging.
Hibernate used for all data access.	Generic Dao using Hibernate used for data access.
Log-in/Authentication is still needing to be added. Once implemented, it will add value by limiting who can upload new poems	Authentication implemented and adds value to the application.
Consumes at least one web service or public api using Java.	Consumes Amazon OCR resource for converting pictures with handwriting to text. Impressive!
Update and Delete functionality still need to be added	Application is database-driven using full CRUD.	Create and Read funcitonality appear good!
Database includes multiple one to many relationships.	Includes a many to many relationship
Deployed to AWS for public access.	A version of the application is deployed to AWS
Properties file could be used for some hard coded values (e.g. bucket-name), it appears that this is currently planned based on the TODOs	Implements best practices (for example, data validation). It also doesn't appear that the file upload size is checked? - it's possible that I am missing that somewhere.
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.	This is a really fun and creative project. It was cool to see
Implemented technologies or techniques not covered in course materials.	Using the AWS service for character recognition was very impressive!
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	Methods have distinct, single purposes and are well documented with javadoc.
Well-structured project	Well structured MVC architecture
Descriptive naming of packages, classes, methods, variables	The Controllers are named descriptively. I am able to accurately guess the purpose of classes before looking at the code.
Classes appropriately-sized (no monster classes)	Classes are appropriately sized! No classes are enormous
CPD (copy paste detection, meaning are the same lines of code repeated?	I do not see repeated code as I am looking though the project classes. The presentation layer is nicely separated, with the head, header, and footer reused across multiple pages!
Are there candidates for super/subclass relationships, abstract classes, interfaces	No candidates for abstract classes, etc.	Various examples (e.g. PoemAddAction line 62) - however, they are marked with TODOs	are any values hard-coded that should be in a properties file?
Proper exception handling	Logging framework used to log errors. Possible exceptions handled in try catch blocks. Resources closed after used (e.g. S3.java line 65)
Proper error reporting to the user - custom error pages?	A custom error page is implemented.
Code documentation	Code is well documented though javadoc and occasional comments (e.g. Textract.java line 100). Variable names are largely self-explanatory.
Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods?	doGet and doPost methods are straightforward. More coplex logic has been moved to other methods within the classes.
Evaluate the JSPs for templating, data validation, overall look and feel.	Templating was used very well in the JSPs. The overall look and feel is very professional and streamlined.
JSPs use JSTL and EL, no java code	JSTL and EL are used. No java code present
Unit tests are truly a unit test rather than a high level functional test	The individual methods in the Daos are tested.
Test data is appropriately cleaned up or handled	@BeforeEach method used to reset database after each test
Tests could confirm that when a poem is deleted, the genres associated with it are not deleted, and vice versa	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	@BeforeEach used to eliminate redundant code.
Other comments/notes?
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.
What does github history and insights demonstrate about the work on this project?	Steady work is shown in the repository timeline. Github issues show incorporation of feedback on project.
Project complexity
Consider the user display exercise to be a simple project; how does this project compare?	This project is substantially more complex than the user display exercise.
What aspects of the project add complexity?	Utilizing the Amazon OCR added a lot of complexity.
Additional Comments

Overall this is a very impressive project. Most of the work to be done simply involves building out the rest of the CRUD functionality and incorporating a properties file to reduce hard-coded values. With respected to completed code, I did notice that the dao could be tested to confirm that deletion of a poem doesn't unintentially delete genres (and that deletion of a genre doesn't delete the associated poems).

I really enjoyed reviewing your project and look forward to seeing the finished application!

@redfoxanna @pawaitemadisoncollege

[pawaitemadisoncollege]

Hi @MNuenninghoff - thanks for sharing your thorough review of @redfoxanna 's project. Your review helps clearly identify areas to focus on in the remaining time this semester, helps confirm what is complete, and based on the level of detail provided (listing specific classes and line numbers, for example), shows that you took your time to carefully review the project and provide meaningful feedback. Your skills here should serve you well in a dev role.