kmracchini / stitchable

Pattern searching application for cross stitchers
0 stars 0 forks source link

Peer review feedback #6

Closed jcmann closed 2 years ago

jcmann commented 2 years ago

Design/Code Review 2

Project: Stitchable

Developer: Kristin

Reviewer: Jen

Areas for Improvement Criteria Items Met or Exceeded
Project effectively utililizes the technologies and technques specified in the project objectives
Planned MVP Functionality X
Logging framework used, i.e., no System.out.println() or printStacktrace() statements. X
Hibernate used for all data access. X
Just remember to make sure signing out works! Authentication implemented. X
Consumes at least one web service or public api using Java. X
Application is database-driven using full CRUD. X (Good blending of API data + database data)
Database includes multiple one to many relationships. X
Not yet, but soon Deployed to AWS for public access.
We realized we were both a bit confused here, but a lot seems to be fine thanks to Java built-in functionality? 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. X Your code is not only really thorough, but you do push outside of what we do in class! Especially figuring out the many to many thing, and your SQL situation!
Implemented technologies or techniques not covered in course materials. X Yay for project lombok especially!
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 X
Well-structured project X
Descriptive naming of packages, classes, methods, variables X
Classes appropriately-sized (no monster classes) X
CPD (copy paste detection, meaning are the same lines of code repeated? X I think you're being really thorough about this, which especially was obvious when you were talking about maybe making that semi-duplicated table one thing. I think your code looks super clean and you're very aware of avoiding this! I especially think centralizing the loadProperties() would be a good quick thing to do.
Are there candidates for super/subclass relationships, abstract classes, interfaces X
You should move your API key into properties! are any values hard-coded that should be in a properties file?
You already have a few todo's for error handling, but what you have looks good. Proper exception handling
Proper error reporting to the user - custom erro pages? X
Code documentation X
Is there code in the servlet doGet/doPost that should be refactored into testable classes or methods? X
Evaluate the JSPs for templating, data validation, overall look and feel. X Seems good! Re: whether you need to make your table templated, I really think you're good to leave it for hypothetical scalability, and also being able to make that page extra unique by working with the table.Good use of bootstrap, too! :)
JSPs use JSTL and EL, no java code X
Unit tests are truly a unit test rather than a high level functional test X
Test data is appropriately cleaned up or handled X
There is full coverage of methods that perform business logic or utility functions X Seems thorough!!
Redundant code is eliminated by using set up and tear down methods, i.e., @Before, @After X I wonder if you'd need an @After? I don't think so because of how your SQL reset works?
Other comments/notes? Your code is very thorough and well-organized!
Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) to meet milestones and project objectives. Definitely!
Evidence of significant revision and incorporation of feedback. Also definitely!
Project complexity You meet all the rubric marks and you've added other tools like project lombok, so I think you meet this :)
Additional Comments
kmracchini commented 2 years ago

@pawaitemadisoncollege - just making sure you saw this!