MadJavaEntSpring2017 / assist

0 stars 0 forks source link

Design/Code Review 2 - Week 8 #3

Open Iandris opened 7 years ago

Iandris commented 7 years ago

Design/Code Review 2

Project: Assist

Developer: Ben Nisler

Reviewer: Mike Young

Category Criteria Analysis/Comments
Running application
Describe the application functionality that has been built and is working. login page exists, and authentication is functional. 2 lists are populated on different pages
What is good? sucessfully deployed to AWS and functional authentication
What could be improved? no styling on the page yet
Unit Tests
What are the code coverage statistics? Test classes exist for a Dao and a controller entity
Are @Before/@BeforeClass/@After/@AfterClass used to reduce redundant code? @Before is present, do not see an @After
JSPs
Is templating used to eliminate rundandant code(for example, header.jsp, footer.jsp, etc.) yes, the head element is in its own .jsp file
What is good? good use of EL to display list items
What could be improved? nothing yet, I dont see any components or elements that are empty or unused.
Code quality
Are methods single-purpose? yes, methods are small and single purpose
Are classes appropriately-sized classes (no monster classes)? classes are not oversized
Are the same lines of code repeated at all? no repeition found
Do any classes perform very similar functions that could be candidates for super/subclass relationships? DAOs are a little repetative as expected
Are any values hard-coded that should be in a properties file? none found
Are variable names descriptive? yes, variable names are concise
Are there many branches or loops that could be simplified or broken up into smaller methods? In LeagueDao.java the getAll leagues method has a nested for loop. Once hibernate relationships are finished, this can be done differently to remove the nesting
Do the DAOs use Hibernate? No hard-coded sql! yes, hibernate is used in Dao
Has log4J been added? log4J is imported into project
Are there logging statements in the code? log statements are present in test classes
Are appropriate logging levels used? Info, debug, error, for example. yes, log.info i do not see debug or error
Are there any System.out.printlns in the code? no system.out found
Other comments/notes?
Web Service/API integration Which web service/api is being used? Google Maps Api
Is the integration built? not currently implemented
If so, evaluate the integration code. What is good? What could use improvement?
Independent research topic What is the developer's independent research topic? AngularJs and/or Spring
Has the topic been implemented in the project? Spring is implemented
If so, evaluate the implementation. What is good? What could use improvement? not able to make a judgement on implementation of Spring as I dont understand it myself
Key takeaways List at least 3 things you learned while reviewing this project that will help you in your own project. I like the annotation to simplify get/set methods with Spring