Peer review

[chstudebaker]

@pawaitemadisoncollege @JBostroem96

Code Review - Project: Indie Project

Developer: Jimmy Bostroem

Reviewer: Cole Studebaker

---

Areas for Improvement	Criteria	Items Met or Exceeded
	The project effectively utilizes the technologies and techniques specified in the project objectives
	Planned MVP Functionality
	Logging framework used, i.e., no System.out.println() or printStackTrace() statements.	Met: Jimmy utilized Log4j. However, there's a need for further refinement in logging implementation.
	Hibernate used for all data access.	Met: Hibernate is implemented in Beans.
	Authentication implemented and adds value to the application.	Partially Met: Authentication is implemented with a login feature. However, there's room for improvement in preventing access to restricted pages without logging in.
	Consumes at least one web service or public API using Java.	In Progress: Initial setup complete, but further integration is required.
	Application is database-driven using full CRUD.	Partially Met: CRUD functionality implemented on some aspects, but completeness across all entities is needed.
	Database includes multiple one-to-many relationships.	Met: Includes many-to-many and one-to-many relationships.
	Deployed to AWS for public access.	Pending: Deployment to AWS is planned but not yet completed.
	Implements best practices (e.g., data validation).	Partially Met: Validating form fields and removing System.out.println() statements show adherence to best practices. However, improvements in exception handling and error reporting are needed.
	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.	Exceeded: Jimmy's over a hundred commits and consolidation of JSPs demonstrate initiative and originality.
	Implemented technologies or techniques not covered in course materials.	Met: Implements Bootstrap and plans to use enumerations.
	Code Quality
	Single-purpose methods.	Met: All methods accomplish a single purpose.
	Well-structured project.	Met: Naming and structure are straightforward and easy to follow.
	Descriptive naming of packages, classes, methods, variables.	Met: Packages, classes, methods, and variables are appropriately named.
	Classes appropriately-sized (no monster classes).	Met: Class sizes are reasonable.
	CPD (copy-paste detection, meaning are the same lines of code repeated?).	Met: Uses generic DAO and before/after hooks effectively.
	Are there candidates for super/subclass relationships, abstract classes, interfaces?	Met: Uses properties files instead of hard-coded values and implements generic DAO interface.
	Proper exception handling.	In Progress: Working on improving exception handling, currently uses contains method but aims for full try/catch implementation.
	Proper error reporting to the user - custom error pages?	Pending: Custom error pages not yet implemented, but planned for the future.
	Code documentation.	Needs Improvement: Encourage thorough documentation of methods, classes, and variables.
	Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods?	No major issues: Most code is properly factored, with some adjustments needed.
	Evaluate the JSPs for templating, data validation, overall look and feel.	Met: JSPs look good and utilize Bootstrap for a nice look.
	JSPs use JSTL and EL, no Java code.	Met: Removed include statements and replaced them with import statements.
	Unit tests are truly unit tests rather than high-level functional tests.	Partially Met: Still working on Delete Test, but other tests run correctly.
	Test data is appropriately cleaned up or handled.	Met: Tests use cleanDB.sql to clean out the database each time.
	There is full coverage of methods that perform business logic or utility functions.	In Progress: Still in development.
	Redundant code is eliminated by using set up and tear down methods, i.e., @Before, @After.	Met: Uses @BeforeEach effectively.
	Other comments/notes?
	Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) to meet milestones and project objectives.	Exceeded: Detailed commit history and insights demonstrate good project management.
	Evidence of significant revision and incorporation of feedback.	Met: Commendable commitment to revision and feedback incorporation.
	What does GitHub history and insights demonstrate about the work on this project?	Met: Detailed commit messages and 120+ commits demonstrate a good commit regimen.
	Project complexity.
	Consider the user display exercise to be a simple project; how does this project compare?	Exceeded: Reasonably more complicated, with additional features beyond the user display exercise.
	What aspects of the project add complexity?	Noted: Hibernate implementation and specific ID elements contribute to complexity.
	Additional Comments

---

Additional Feedback:

1. Enhanced Logging Implementation: It's great to see the adoption of Log4j for logging purposes. I encourage Jimmy to further refine their logging implementation by incorporating different log levels appropriately. This will help in debugging and monitoring the application more effectively.

2. Completion of Authentication Flow: While authentication functionality has been implemented with a login feature, there's a noted improvement area regarding accessing pages without proper authentication. I recommend focusing on completing the authentication flow to ensure that users cannot access restricted pages without logging in.

3. Progress on Web Service Integration: I acknowledge the initial setup of consuming a web service or public API using Java. I encourage Jimmy to continue making progress in this area to fully integrate the desired functionality into the application.

4. Refinement of CRUD Functionality: While CRUD functionality has been implemented on some aspects, there's room for improvement to ensure it covers all necessary areas. I suggest prioritizing the completion of CRUD operations across all relevant entities to provide comprehensive data management capabilities.

5. Future Deployment on AWS: It's noted that deployment to AWS for public access is still pending. I encourage Jimmy to continue working towards this milestone as it will provide valuable experience in deploying applications to cloud platforms.

6. Ongoing Work on Code Quality and Best Practices: I highlight Jimmy's efforts in implementing best practices such as data validation and removing System.out.println() statements. I encourage them to continue refining their code quality by focusing on areas like proper exception handling, error reporting, and utilizing properties files for configuration values.

7. Continuous Learning and Experimentation: I recognize Jimmy's initiative in exploring and incorporating technologies and techniques beyond the scope of the course materials, such as Bootstrap and planned usage of enumerations. I encourage them to maintain this proactive approach to learning and experimentation throughout the project.

[pawaitemadisoncollege]

Hi @chstudebaker! Thanks for your thorough review of @JBostroem96's project.

In some cases, you provided very specific detail, such as the number of commits. In other cases you have included more detail to bring better focus to the area for improvement. For example, "Most code is properly factored, with some adjustments needed." Where are adjustments needed? I wouldn't expect you to list them all, but a single example would be nice to give the developer a specific "thing" to do. For example, I noticed there are still a few printstacktraces hanging around that should be replaced with logging statements: https://github.com/search?q=repo%3AJBostroem96%2FindieProject%20print&type=code.

I don't expect you to make any revision to this, instead, it is feedback to consider for future code reviews.

Last thing - did you use any of the code quality plugins or tools to assist with this review? If so, it's often helpful to include a copy of the report highlighting some of the "easy wins/easy fixes" for developer.