billyJoePiano / TenaPull

TenaPull is a configurable Java application which fetches and processes the data from one or more Nessus APIs, and converts it into JSON ouputs that are usable by Splunk
7 stars 1 forks source link

Peer Review #10

Closed qtgiebel closed 2 years ago

qtgiebel commented 2 years ago

Design/Code Review 2

Project: NessusTools

Developer: Bill Anderson

Reviewer: Quinn Giebel

Areas for Improvement Criteria Items Met or Exceeded
Project effectively utililizes the technologies and technques specified in the project objectives
Planned MVP Functionality Splunk integration appears functional. I would suggest removing the dashboard and active directory authentication from the MVP as it appears the scope has changed to no longer include them. Also consider discussing how the scope has changed with @pawaitemadisoncollege. We also discussed putting together a 'marketing' demo, but it sounds like that may be either cost or security prohibitive. X
Logging framework used, i.e., no System.out.println() or printStacktrace() statements. Logging is functional however there are a few instances of stacktraces and printlns. https://github.com/billyJoePiano/NessusTools/search?q=printStacktrace https://github.com/billyJoePiano/NessusTools/search?q=println X
Hibernate used for all data access. Yes. X
Authentication implemented. There is not. --
Consumes at least one web service or public api using Java. Consumes the Nessus API, however I do not know if this would be considered a web service or public api. ?
Application is database-driven using full CRUD. The application is database driven and create and read have been implemented. Delete and update are still needed. X
Database includes multiple one to many relationships. So many. Like, so many. X
Deployed to AWS for public access. It has not been, but the scope of the application may not support a public deployment. --
Implements best practices (for example, data validation) The scope and scale of this project is a bit beyond me but I do believe that best practices have been followed. The use of nested classes is particularly clever, and in my opinion implemented well. X
Synthesis of multiple concepts in unfamiliar situations requiring research beyond the scope of the class
Experiments individually, exhibits independence and drive, shows originality in the solution. Absolutely. The depth of data objects on display is honestly impressive. X
Implemented technologies or techniques not covered in course materials. Yes, your use of nested classes is very interesting. It looks like the project has delved much deeper into the intricacies of Hibernate than we covered in class. X
Code quality - Evaluate 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 Methods are well scoped. X
Well-structured project You might consider adding more verbose packaging to indicate how the java classes are related to each other, although on further inspection it appears the nested classes may do that for you. X
Descriptive naming of packages, classes, methods, variables Naming is very descriptive. X
Classes appropriately-sized (no monster classes) Classes are reasonably sized. X
CPD (copy paste detection, meaning are the same lines of code repeated? Using the Intellij code analysis tools I did find some repeated code. You can run the duplicate code scan from the CODE menu in Intellij. Code -> Analyze Code -> Locate Duplicates image --
Are there candidates for super/subclass relationships, abstract classes, interfaces Your inheritance structure is comprehensive. X
are any values hard-coded that should be in a properties file? None that I could find. X
Proper exception handling Exception handling is good X
Proper error reporting to the user - custom error pages? Error reporting is well handled in the logs. X
Code documentation It appears documentation has not been add yet. --
Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods? There are no servlets present. --
Evaluate the JSPs for templating, data validation, overall look and feel. There are no JSPs present. --
JSPs use JSTL and EL, no java code There are no JSPs present. --
Unit tests are truly a unit test rather than a high level functional test Unit tests are well formed. X
Test data is appropriately cleaned up or handled Test data is well handled. X
There is full coverage of methods that perform business logic or utility functions I was not able to test the code coverage. --
Redundant code is eliminated by using set up and tear down methods, i.e., @Before, @After Tear down methods are implemented thoroughly. X
Other comments/notes? This project is quite complex seems very comprehensively done. Unfortunately, I wasn't able to run it myself, but did see the inputs and output, and they show a lot of time and dedication spent on this project. X
Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) to meet milestones and project objectives. The planning phase was well thought out. It does appear there were some instances in which material reality forced you to deviate from the plan, but you seem to have adjusted well. X
Evidence of significant revision and incorporation of feedback. The issue history shows several instances of iteration and responses to feedback. X
Project complexity Project is very complex especially in terms of object X
Additional Comments This project is very cool and I hope the deployment goes well. X