Code Review

[bnisler]

Design/Code Review 3

Project: myCine

Developer: Mike Iandris

Reviewer: Benjamin Nisler

Category	Criteria	Rating/Comments
Project Overview
Which planned functionality has been met?	Able to add, remove movies from wishlist, library, add and remove friends, rentals, use IMDB API, sends email and text message alerts
What planned functionality has not been met?	na
Describe the GitHub history and what it demonstrates about the project progress during the semester.	lot of work spent between January and mid-February, tapers off at the end of February with minimal activity during March, likely due to team project work. Has a spike during the 1st couple of weeks in April. Seems like Mike has been consistently putting time into his project.
Describe how peer and instructor feedback/recommendations were incorporated into the project?	Broke up some monster classes/methods into more manageable classes/methods
Other comments/notes?
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel.	Templating seen throughout, good use of expression language to display data, didn't see any business logic on the frontend, kept it in the backend. Validation performed using Bootstrap's form-control. Looks/feels like a professionally done app, good use of graphics.
Other comments/notes?
Java code quality	Evaluate the code quality for the following and identify specific areas for improvement (class, method or line number) single-purpose methods well-structured project descriptive naming of packages, classes, methods, variables classes appropriately-sized (no monster classes) CPD (copy paste detection, meaning are the same lines of code repeated are there candidates for super/subclass relationships, abstract classes, interfaces? are any values hard-coded that should be in a properties file? proper exception handling proper error reporting to the user code documentation	For the most part, single-use methods are incorporated, refactored a large one into smaller "validator" class with methods, directory/package is structured well, I like the way you've broken up the navigation-related servlets and the functional ones. Names depict/convey meaning to what's being done and is descriptive. Only areas of duplication I saw were in the DAOs and a bit in the REST service. Only area I saw hard-coded values were in the API integration. Code documentation needs updating. Several places of commented out code. See TODO's
Logging	Evaluate the use of logging, for example: appropriate use of logging statements in the code for error logging and debugging logging levels used - info, debug, error no occurrences of System.out.printlns or printStackTrace() logging works on Digital Ocean deploy	log statements not found. a few .stackTrace() methods found, didn't see any System.out.printlns. See TODO's
Unit Tests	Evaluate the unit tests, for example: tests are truly a unit test rather than a high level functional test test data is appropriately cleaned up or handled 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	Good use of @Before and @After in tests to reduce redundancy. Exceptions and assertions were present, tests covered the DAOs and their methods.
Security	Evaluate authentication/authorization:
Is it implemented correctly and working locally as well as on Digital Ocean?	Demonstrated functionality is implemented and working. Incorporated 'admin' functions.
Web Service/API integration	Evaluate the service/api integration, for example: Which service/api is implemented? how is error handling of the service implemented? For example, What happens if the service is not available?	IMDB API incorporated as well as his own restful "Titles" service. Exceptions are caught, but didn't see anything being pushed to notify the user.
Other comments/notes?
Independent research topic	What is the independent research topic?	Groovy unit testing.
Is the independent research topic/technique implemented in the project?	Used to test email messaging service.
Other comments/notes?
Deployment	Has the application been successfully deployed to AWS or another hosting service?	Yes
Is the hosted application fully functioning?	Yes
Other comments/notes?

[bnisler]

pushed up another branch "code-review" that has TODO comments in them.

Just kidding, I don't have write permissions. Let me know if you'd like me to push those out.

[Iandris]

Found the "massive" section of commented out code and removed it. This was from extracting the UserCreation and Validator classes. Thanks for the heads up.