Peer code review #2 Beth

[bethscholze]

Developer: Anthony

Reviewer: Beth

Category	Criteria	Rating/Comments
Project Overview
Which planned functionality has been met?	user authentication, invoice generation, invoice to pdf api integrated
What planned functionality has not been met?	admins ability to give users permission to see certain clients(update permissions servlet is what I have written down), implement the frontend for converting invoices to pdf(download invoices page), dashboard work
Describe the GitHub history and what it demonstrates about the project progress during the semester.	Remember to update your link in the student repo to your new project(InvoiceGenerator). Looking between both the old project and new one it looks like you have been making steady progress on your project, great job!
Describe how peer and instructor feedback/recommendations were incorporated into the project.	-I believe you said you did a db design document, but had not put it on github, I think you should, employers would love to see that. It looks like you incorporated all of Elspeths feedback except logging. If you plan on adding in lombok it is even easier to do log statements(add in logging, it makes debugging so much easier!)
Other comments/notes?
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel.	These looked great! I think you need some validation still, but I can tell you definitely put a lot of time in getting that generate invoices page looking nice.
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	using sonarLint since you are already looking through your project with QAPlug

your code looks pretty clean already

you have some larger classes, but I think that is fine in this case since it is styling for the invoices

could make some functions private/protected in the PDFInvoicesBasic class since the main function calls all the other functions

sonarlint suggests try with resources in that class too

make sure you add brackets where qaplug tells you to for for loop and if loop

check for values that can be constants

in CreateUser @override the doget method | |Category|Criteria|Rating/Comments| |--------|---------|---------------| |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|System.outs in Database class|Other comments/notes?| very little logging | |**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 ||Other comments/notes?| All the daos are tested. Still need tests for other methods. | |**Security**|Evaluate authentication/authorization:| | ||Is it implemented correctly and working locally as well as on AWS? ||Other comments/notes?| Can't test it, no local database and aws down. | |**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?| Itextpdf implimented, there isn't error handling. | ||Other comments/notes?| | |**Independent research topic**| What is the independent research topic?| mockaroo | ||Is the independent research topic/technique implemented in the project?| no | ||Other comments/notes?| | |**Deployment**| Has the application been successfully deployed to AWS?| probably | ||Is the hosted application fully functioning?| aws down | ||Other comments/notes?| * Make sure to fix the link in the student repo so it points to the new version of your project |

[bethscholze]

Sorry this looks so messed up in the middle, I couldn't get the markdown to work correctly and I just don't have it in me to keep messing with it. Also I hate using markdown