Charles' feedback

[CharlesClark84]

Design/Code Review 3

Project: Music show finder and ride share

Developer: Kevin Heise

Reviewer: Charles Clark

Category	Criteria	Rating/Comments
Project Overview
Which planned functionality has been met?	Add new user, login, basic jsps to view info
What planned functionality has not been met?	full crud, api call to get upcoming shows
Describe the GitHub history and what it demonstrates about the project progress during the semester.	lots of gaps in commits could have spent more time throughout semester instead of trying to get all done at the end.
Describe how peer and instructor feedback/recommendations were incorporated into the project.	added generic dao, controller and entity directories, checked off finished tasks
Other comments/notes?	a few objectives have been missed on the weekly checklists
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel.	good data validation, looks great so far very visual
Other comments/notes?	files are well organized in separate folders
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	Code looks great so far, have done a great job of keeping it nice and clean	Other comments/notes?	Code looks really neat and well organized, some small duplicate code issues that are discussed in comments below
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	Other comments/notes?	no use of system.out print lines and logging is there for classes that have been created
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 logging statements, no system.out print lines seen, make sure all necessary classes have log statements	Other comments/notes?	UserDao test looks good, make sure all classes have tests written for them that need them
Security	Evaluate authentication/authorization:	authentication looks correct
Is it implemented correctly and working locally as well as on AWS?
Other comments/notes?	think there is authentication issues on AWS
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?	Service not implemented yet
Other comments/notes?	call to retrieve a list of future music shows and different venues around the state
Independent research topic	What is the independent research topic?	Not sure what topic is, maybe add a list of what these topics are in your readme.md
Is the independent research topic/technique implemented in the project?	I like the SASS hamburger menu
Other comments/notes?	website looks great so far and very original, spoke off social network like part of application where you can add friends, view profiles, and send messages
Deployment	Has the application been successfully deployed to AWS?	no link provided but pulled up in class on AWS
Is the hosted application fully functioning?	log on issues on AWS, check database and that all files that your need are on your EC2 and in correct location
Other comments/notes?	cant wait to see it demoed, used QA plug to check src code

[pawaitemadisoncollege]

@CharlesClark84 Which code quality tool from week 14 did you use to analyze the code? What items did the code quality tool highlight?

[CharlesClark84]

indieproject Maintainability Avoid Duplicate Literals SignUpUser The String literal "errorMessage" appears 6 times in this file; the first occurrence is on line 60 The String literal "signUp.jsp" appears 6 times in this file; the first occurrence is on line 61 Unused local variable Reliability Malicious code vulnerability - May expose internal representation by incorporating reference to mutable object User EI2: new com.kevinheise.entity.User(String, String, String, String, String, Date, String, String) may expose internal representation by storing an externally mutable object into User.birthDate EI2: com.kevinheise.entity.User.setBirthDate(Date) may expose internal representation by storing an externally mutable object into User.birthDate Malicious code vulnerability - May expose internal representation by returning reference to mutable object Usability Dodgy - Dead store to local variable GenericDao DLS: Dead store to root in com.kevinheise.persistence.GenericDao.getAll() UserDao DLS: Dead store to root in com.kevinheise.persistence.UserDao.getAll() If Stmts Must Use Braces Role Avoid using if statements without curly braces Avoid using if statements without curly braces Maintainability- You could create a properties file for these values Reliability- reduce risk point by making objects only changeable within class
Usability- Create a reference variable that holds this value and call it in your code Usability- Add curly braces to if statements

[pawaitemadisoncollege]

@kheise91 Looks like some good suggestions here, be sure to address some of those code quality findings :)