swansonj2019 / PersonalMovieDatabase

A Personal Movie Database Application
0 stars 0 forks source link

Yia's Review of Personal Movie Database #6

Open yiax opened 4 years ago

yiax commented 4 years ago

Design/Code Review 3

Project: Personal Movie Database

Developer: Joel Swanson

Reviewer: Yia Xiong

Category Criteria Rating/Comments
Project Overview
Which planned functionality has been met? Currently, the application hanles adding a new user, adding a new movie, and searching for movies.
What planned functionality has not been met? Joel has additional function from his user stories that he has not implemented or may not implement since they may not be consider "MVP"; funciton such as being able to delete ratings for a movie, add a rating for a movie, view recently added movies.
Describe the GitHub history and what it demonstrates about the project progress during the semester. The GitHub history shows consistant progress and continuous activities. The commit comments shows his progress from adding initial test, adding his dao and dao test, showing passing of certin dao test, implementating authentication, adding api to his application, building REST services, to building his front end feature.
Describe how peer and instructor feedback/recommendations were incorporated into the project. It's hard to tell if any feedbacks were applied as his appication grows. There are a few open issues that has not been closed.
Other comments/notes?
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel. The JSP are clearly commented and it's easy to tell which block of code represent which element on the webpage. Each JSP serves the business use of the page, for example, the login.jsp allows user to login, the search.jsp allows for user to search for movies. I would recommend breaking off repetative codes into a seperate jsp and including them in them in all other jsp; for example, the head tag is repeated on mutiple page and can posibility be broken into its own jsp, the navigation menu. This will make maintaining the webpage easier in the future.
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
  • The code are pretty easy to read. I would recommend condensing his DAOs into a generic DAO since a lot of them are repetative code. The entities are properly created with no arguement constructor, arguement constructor, and setters and getters for his instance variables. So far, his controller are serving it proper purposes in adding movies or performing a search before directing back to the jsp. I don't see any hard code values as he's either accessing data from a property or from a request. Other comments/notes? ***
    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 AWS deploy
  • Joel's code uses logger statements opposed to print statements throughout his controller and presistence classes. Approperiate debug were used in his DAO class as informational events if something goes wrong. He also uses error level logger message in this try statements to designate error events. In his controller classes, he added info level logger to trach and show where his program is during the run. Other comments/notes?
    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
  • Most of the testes were commented out as Joel had mention he needed additional work on the test. If the test are not passing, it could be either a database issue or hibernate nontation on the entities. Try testing one test at a time and resolve one issue at a time.
    Other comments/notes?
    Security Evaluate authentication/authorization: At the moment, it's difficult to tell if autentication is working. Based on Joel's code, it appears he had set up his context and web xml properly to allow for proper autentication and authorization to certian pages. However, the database create resource file doesn't indicate that a user role table has been created. It could be that the database resorse file has not been updated. Since I was not able to access his applicaiton on AWS, I was not able to validate if it is properly autheticateed in AWS.
    Is it implemented correctly and working locally as well as on AWS?
    Other comments/notes?
    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?
  • Joel is using The Movie Databases's API to get movie data for his web application. I don't see any error handling at this point. It will be nice to add front end solution if incase the service is not available to inform users of an issue.
    Other comments/notes?
    Independent research topic What is the independent research topic? Joel's individual research topic is using Chatbot with Amazon Lex
    Is the independent research topic/technique implemented in the project? At this point, it does not look like this was implemented in his web application.
    Other comments/notes?
    Deployment Has the application been successfully deployed to AWS? It appears Joel has deployed to AWS but I'm having trouble accessing the website.
    Is the hosted application fully functioning? I can't tell if the application is working since I'm unble to access the website.
    Other comments/notes?