search

abriggs3 / snagLog

web application for documenting tree falls and snags in paddling routes
0 stars 1 forks source link

Peer Review 3 #8

Open ssoperMadisonCollege opened 6 years ago

ssoperMadisonCollege commented 6 years ago

Design/Code Review 3

Project: SnagLog

Developer: Aaron Briggs

Reviewer: Sam Soper

Category Criteria Rating/Comments
Project Overview
Which planned functionality has been met? Authentication, database structure, Most unit testing (DAOs) is done.
What planned functionality has not been met? Needs an offical java consumable API. Need ssl cert individual research topic implemented.
Describe the GitHub history and what it demonstrates about the project progress during the semester. Aaron has clearly worked on this project consistently throughout the semester based on the git commit history.
Describe how peer and instructor feedback/recommendations were incorporated into the project. Aaron is still working on closing an issue or two, but that is on his radar. He has closed out to several issues on github, incorporating feedback from the instructor and peers.
Other comments/notes? The SnagLog looks incredible.
JSPs
Evaluate the JSPs for templating, business logic, data validation, overall look and feel. JSPs are well organized, jstl is used well. One thing to consider would be to separate some of the html from the jstl to clean up the look of the code a little.
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
    		•
  • Methods and classes are compact and easy to follow, nice work here. Nothing screams "break this up" to me.
  • Project structure follows the same patterns we've learned in class, looks great.
  • Everything is well-named.
  • No monster classes.
  • No CPD that I see.
  • The generic dao takes care of most repeated code in the application. One suggestion for the future would be to use something like project Lombok to reduce lines of code in the javabeans.
  • Most things that should be in a properties file are in a properties or config file.
  • There is some exception handling in JavaScript. Might be good to have generic 404, 500, etc. error pages defined in the web.xml.
  • There are success and failure messages when the user fails validation for logging in and signing up, looks good.
    		•  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
    		•
  • There are opportunities to add more logging, but there is logging where there was troubleshooting taking place. I think this is generally where you want logging.
  • Logging levels are mostly error-level.
  • No occurences of s-outs or print stack traces, logging works on AWS.
    		•  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
    		•
  • Aaron makes use of the generic dao, which takes care of all his dao unit testing. There is opportunity to add unit testing, however, it would require testing javabeans, servlets, etc, which would require mock java objects and probably isn't worth it.
  • There isn't much business logic to test via java, but there may be in the future with API/independent topic implementation.
    		•
    Other comments/notes?
    Security Evaluate authentication/authorization: It is air-tight. There is multi-level security that discriminates between the admin and contributer roles.
    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?
    		•  A work in progress.
    Other comments/notes?
    Independent research topic What is the independent research topic? A work in progress.
    Is the independent research topic/technique implemented in the project?
    Other comments/notes?
    Deployment Has the application been successfully deployed to AWS or another hosting service? Yes
    Is the hosted application fully functioning? No
    Other comments/notes? No
    ssoperMadisonCollege commented 6 years ago

    Looks good, Aaron! Good luck in these next 2 weeks!