Good: Sign up, sign in, add game, add game manually, delete game. Needs Work: view my games; there's the ability to search through your games based on title, but no filtering based on platform. No information beyond a welcome message when viewing profile.
What planned MVP functionality has not been met?
There is no option to edit the profile
Logging framework used, i.e., no System.out.println() or printStacktrace() statements.
Good: log4j used and almost no system.out.println()/printStackTrace(). Needs Work: logging not very utilized. Loggers missing on some dao methods. Database.java still has System.out.println()
Hibernate used for all data access.
Good: Hibernate is used for all data access
Authentication/Authorization implemented.
Good: Tomcat authentication is set up. Needs Work: The only time authorization is used is for the admin role and the ability to search users with search.jsp.
Consumes at least one web service or public api using Java.
Needs Work: Web service/api not used at this point
Application is database-driven using full CRUD.
Good: Full CRUD application
Database includes multiple one to many relationships.
Good: There are multiple One to Many relationships
Deployed to AWS for public access.
Needs Work: The AWS link was not available in the Student repository, but at the time of the code walk-through on Tuesday, The application was not working on AWS
Implements best practices (for example, data validation - front end and back end)
Needs Work: No data validation when adding users or games
Describe the GitHub history and what it demonstrates about the project progress during the semester.
There were regular commits from Jan 31 - March 7 and then it appears that no progress was made until the week of April 25th
Describe how peer and instructor feedback/recommendations were incorporated into the project.
Checkpoint 2 - Good: Role table was set up for authentication. Needs Work: include directives not swapped out for jstl import tags in jsps. Still has entity specific DAOs instead of a generic DAO.
Other comments/notes
It was a little difficult to evaluate the code without being able to go back and look at the working code on AWS
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel, responsiveness, accessibility.
The business logic on the JSPs looks good. There isn't much styling at this point, so the look and feel isn't great. Accessibility needs work, some of the inputs are missing labels and make sure you're using alt text on your images.
Other comments/notes?
Java code quality
Evaluate the 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
In most of the servlet classes, there is logic that could be broken out into separate methods.
well-structured project
The only suggestions I would make for the project structure is to put the controller, entity, persistence, etc... all under the same package. Example (from the user display exercise): edu.matc is the package that contains entity (edu.matc.entity) and controller (edu.matc.controller), etc... and to maybe organize your jsps into folders, just because there are so many of them.
descriptive naming of packages, classes, methods, variables
The names on some of the JSPs made it a little hard to figure out what their purpose was and what they connected to. For example: addgame.jsp and addMyGame.jsp. Comments in those jsps would also make the difference clearer
classes appropriately-sized (no monster classes or methods)
The classes and servlets are well broken up
CPD (copy paste detection, meaning are the same lines of code repeated?)
There is a lot of repetition in the DAO classes that could be eliminated with a generic DAO
are there candidates for super/subclass relationships, abstract classes, interfaces?
Not that I can tell
are any values hard-coded that should be in a properties file?
Database.java uses the database.properties file. I did not find any other hard coded data that should be in a properties file
proper exception handling
Proper exception handling is used
proper error reporting to the user
There is no error reporting to user, they are just sent back to the same page with no message that it didn't work. Example: in the AddGame servlet, if there's an exception, the user is just redirected back to /addgame.jsp with no way of knowing whether or not adding the game worked
code documentation
Not much documentation beyond generic javadoc comments.
is there code in the servlet doGet/doPoost that should be refactored into testable classes or methods?
As mentioned earlier, there is logic in most of the servlets that could/should be refactored into separate methods
Unit Tests
Evaluate the unit tests, for example:
tests are truly a unit test rather than a high level functional test
Yes. The tests look well designed
test data is appropriately cleaned up and reset
Makes use of a cleandb.sql file that clears and resets the database before each test
there is full coverage of methods that perform business logic or utility functions
Some test methods are empty and not all of the methods in each dao have a test
redundant code is eliminated by using set up and tear down methods, i.e., @Before, @After
Set up methods are used. Tear down methods are necessary as far as I could tell
Other comments/notes?
I highly recommend getting the QA plug in that Paula suggested in the week 14 materials. I think that will help a lot with cleaning things up a bit. Overall it's a well designed program and a really fun idea. Just needs to be cleaned up and fleshed out a bit more.
Design/Code Review 2
Project: Video Game Collection
Developer: Noah Aguilera
Reviewer: Amber Custer