pknibbe / repertoire

Enterprise Java project: music manager
0 stars 0 forks source link

Code Review 2 #5

Closed Amatheia closed 7 years ago

Amatheia commented 7 years ago

Design/Code Review 2

Project: Repertoire

Developer: Peter Knibbe

Reviewer: Amatheia

Category Criteria Analysis/Comments
Running application
Describe the application functionality that has been built and is working. Jsps
What is good? looks like a good start
What could be improved? don't need both database and hibernate properties files; finish adding getters and setters so tests work; directory structure could be improved
Unit Tests
What are the code coverage statistics? tests fail, hardly any coverage
Are @Before/@BeforeClass/@After/@AfterClass used to reduce redundant code? Mostly, yes
JSPs
Is templating used to eliminate rundandant code(for example, header.jsp, footer.jsp, etc.) the files are there
What is good? forms look good
What could be improved? not seeing head or footer being used; the mainpanel.jsp pops up on the bottom of other pages - is this a navbar?
Code quality
Are methods single-purpose? seems so
Are classes appropriately-sized classes (no monster classes)? looks like it
Are the same lines of code repeated at all? SessionFactoryProvider in DAO's can be singled out
Do any classes perform very similar functions that could be candidates for super/subclass relationships? none that popped out
Are any values hard-coded that should be in a properties file? check username/password booleans
Are variable names descriptive? Yes
Are there many branches or loops that could be simplified or broken up into smaller methods? Possibly, definitely something to look at once the tests are passing
Do the DAOs use Hibernate? No hard-coded sql! Yes, but missing some code for full implementation of Hibernate
Has log4J been added? Yes
Are there logging statements in the code? Yes
Are appropriate logging levels used? Info, debug, error, for example. mostly info, could use error
Are there any System.out.printlns in the code? None that I noticed
Other comments/notes? could use some more styling to make it look more fun
Web Service/API integration Which web service/api is being used? Will use MyTunes/Amazon Prime
Is the integration built? no
If so, evaluate the integration code. What is good? What could use improvement?
Independent research topic What is the developer's independent research topic? music streaming
Has the topic been implemented in the project? no
If so, evaluate the implementation. What is good? What could use improvement?
Key takeaways List at least 3 things you learned while reviewing this project that will help you in your own project. directories for "engines" and "butlers", use of role and user manager for certain pages, checking role
pknibbe commented 7 years ago

Thanks