subu250 / IndieProjectIconicLiner

Indie Project IconicLiner
0 stars 0 forks source link

Code Review #3

Closed jeliceiri closed 2 years ago

jeliceiri commented 2 years ago

Design/Code Review 2

Project: Iconic Beauty Products

Developer: Subheksha Karki

Reviewer: Jill Eliceiri

Please note: (-) denotes unable to evaluate area as project is work in progress @pawaitemadisoncollege

Areas for Improvement Criteria Items Met or Exceeded
Project effectively utililizes the technologies and technques specified in the project objectives
The project's MVP was explained clearly in the code review meeting on 12/10/21! The MVP could be presented more clearly in repo via problem statement and user stories: this could be improved by matching the screen designs to the user stories. Perhaps revise the problem statement and user stories to match what you presented nicely in the code review meeting. Planned MVP Functionality
Logging framework used, i.e., no System.out.println() or printStacktrace() statements. Log4J is used. Didn't see any System.out.println or printStacktrace
Hibernate used for all data access. Looks like project is work in progress. Hibernate is being used.
Plans are in place to implement Week 7 AWS Cognito in project Authentication implemented.
Plans are in place to consume one public api using Java Consumes at least one web service or public api using Java.
Create user and view products are present in controller. Update and delete are not implemented yet. Application is database-driven using full CRUD.
Looks like there is 1 one-to-many relationship present (user and product). Another is planned (product and location) Database includes multiple one to many relationships.
Plans are in place for week 7 deploy a Cognito app to Elastic Beanstalk Deployed to AWS for public access.
Data validation not implemented yet. Implements best practices (for example, data validation)
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.
- Implemented technologies or techniques not covered in course materials.
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
- 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 - custom erro pages?
- Code documentation
- Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods?
JSP's currently not built out Evaluate the JSPs for templating, data validation, overall look and feel.
- JSPs use JSTL and EL, no java code
One unit test present (UserDaoTest). Need to do for each entity and possibly more. In the tests, you can compare two objects and all their fields by using: assertTrue(object1.equals(object2). To use this, you need to right click in the entity and generate equals() and hashcode() Unit tests are truly a unit test rather than a high level functional test
Test data is appropriately cleaned up or handled cleandb.sql in UserDaoTest
Looks like business logic is work in progress 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 @Before present in UserDaoTest
Other comments/notes?
Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) to meet milestones and project objectives.
Past github issues present with instructor suggestions Evidence of significant revision and incorporation of feedback.
Can improve this area as project is built out Project complexity
-Hide sensitive data such as hibernate.cfg.xml and add it to .gitignore

-Do you need to send email to a user at this point? That could be put on hold for a future version if its not in the MVP. Might be better to focus on the CRUD aspects first.

-It is apparent that you are cognizant of your time constraints due to work obligations, blockers consisting of VM crashing and slowness, and still exhibit drive and intentions to meet milestones.

-It was nice meeting with you, good luck! You can do it! :)
Additional Comments
pawaitemadisoncollege commented 2 years ago

@jeliceiri Thank you SO MUCH for doing a second peer review in week 14. Much appreciated, especially during such a busy time!