Instructor Feedback - Final

[pawaitemadisoncollege]

Affected area/component

None

App version / commit SHA

No response

Environment

Hi @ArchILLtect! Following is my feedback/notes after reviewing your final presentation and reviewing many parts of your code base. In the interest of time I spent more time documenting areas for improvement rather than leaving lots of "kudos". Please don't take that to mean that there isn't a lot of good work here, there is!!!!

Slack feedback - saved here for posterity

Congrats@Nick! You finished CodeForge right on schedule, and have it up on aws w/ auth implemented! Time to celebrate!
You did a fantastic job covering a LOT of ground in your presentation - thanks for being thorough here and making the review process much easier! I really appreciated that you tracked hours so carefully and even included the tech used to do this! Loved the closing quote and your honest evaluation of how things went and why! School should be about practice and learning, not getting it perfect; you made it clear that you grew here! Yay!
Apologies if I missed a couple things in the video (I have not yet looked at code at all, and didn't want to lose track of this):
Question 1: Is there full CRUD in the application? I think I saw view and edit, but curious about create and delete?
Question 2: The db overview went pretty fast, so maybe I missed this. Is there a user table? I think I saw the user name on one of the tables, where I might have expected user id...
Next up: I'll review code and leave some feedback in an issue on your repo. My hope is to have that done by end of Friday!

Additional feedback Note that my comments follow each category/description row.

Category	Exemplary	Meets Expectations	Revision Needed	Not Assessable
Project effectively utilizes the technologies and techniques specified in the project objectives	Logging framework used, i.e., no System.out.println() or printStacktrace() statements. JPA/Hibernate used for all data access. Authentication/Authorization implemented using a third party provider. Provides the ability to register a new user Consumes at least one web service or public api (in addition to those required above) using Java. Application is database-driven using full CRUD. Database includes multiple one to many relationships. Deployed to AWS for public access. Meets the originally defined MVP (minimum viable product) Implements best practices (for example, data validation)	Fulfilled most of the scope.	Fulfilled some significant portions of the project scope.	Barely fulfilled the scope: significant portions are missing.
Instructor Comments	Meets/revision. Deployed to aws, auth implemented, CRU implemented, Quotes api. Fulfilled much of the scope. CSS/Styling of the site gives it a pro feel! Appears to be missing Delete functionality. Missing a user table, currently the username appears to be added directly to tables. Opportunity for better db design/normalization. User experience would be improved: Provide feedback on Drills within the drill page. Currently it seems that the user has to navigate back to the drill list to see feedback Display dates in a more-user friendly format Do not display the cognito userid - it's a long uuid and I don't think it's useful to the user
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 2 or more technologies not covered in class, or 1 complex technology. Demonstrates the ability to independently troubleshoot and debug.	Experiments on own with reliance on instructor or others for guidance. Implemented 1 technology not covered in class.	Attempted with heavy reliance on instructor for recommendations and help.	No observable interest or effort shown.
Instructor Comments	Meets - perhaps to a fault in this case, as you highlighted in your presentation!
Code is readable, documented, efficient and well-tested.	Code is exceptionally well organized and easy to follow. Documentation is well written and clearly explains what the code is accomplishing and how. Code is extremely efficient without sacrificing readability/maintainability. Code has significant test coverage.	Code is fairly easy to follow. Documentation is somewhat useful in understanding the code. Code is efficient. Moderate test coverage.	Code is fairly easy to follow with some exceptions. Documentation is incomplete. Code inefficiencies exist. Some test coverage.	Code is poorly organized. Documentation is simply embedded comments and does not help the reader understand the code Code is unnecessarily long and complex. Little to no test coverage.
Instructor Comments	Some documentation very clearly explains the reasoning/purpose: code-forge/src/main/java/me/nickhanson/codeforge/web/Utf8EncodingFilter.java Lines 11 to 16 in daf9a2a /** * A servlet filter that sets the character encoding for requests and responses to UTF-8. * This ensures that all incoming and outgoing data is properly encoded in UTF-8. * Had to add this filter because Tomcat was not correctly setting UTF-8 encoding on POST requests and I wanted * to be able to use UTF-8 characters in form submissions--specifically emojis. lol * Some code feels overarchitected/unnecessarily complex, such as the evaluation code. In a V1, it's ok to keep things simple to achieve MVP and then grow as needed. It seems like all users would see the same data here? https://github.com/ArchILLtect/code-forge/blob/daf9a2a027e943a23a9a5ba682aa02dac834626a/src/main/java/me/nickhanson/codeforge/persistence/DrillItemDao.java#L59-L72Query doesn't appear to account for the user's history/results. Adding a user table would make it very easy to get the user's drills and filter/sort them: user.getDrills(). As I mentioned in previous feedback, I don't think there's good reason to use HQL in this project. Some classes would benefit from refactoring and documentation. DrillServlet.java for example. There is a LOT going on in this servlet - consider whether it is all necessary or whether there is a simpler way to achieve the goal. For example, follow the code from call to call to finally retrieve the user's drills starting here: code-forge/src/main/java/me/nickhanson/codeforge/web/DrillServlet.java Line 93 in daf9a2a List<DrillItem> items = drillService.getDueQueue(limit, userId); . How many classes and methods are called? Consider this flow instead: after the user logs in, put the user in the session. This way you can simply access user.drills on the jsp, eliminating all those calls through multiple classes and methods. I'd challenge you to make this application as simple as possible while maintaining the current mvp functionality. I am guessing you could cut this to 25% of the existing code or less. Examine code for readability and improve. Example: code-forge/src/main/java/me/nickhanson/codeforge/web/DrillServlet.java Line 165 in daf9a2a javax.servlet.http.HttpSession session = req.getSession(true); . Why fully qualify session here? Import the package for consistency and readability? Why 2? Use a constant that clearly explains this magic number: code-forge/src/main/java/me/nickhanson/codeforge/web/DrillServlet.java Line 178 in daf9a2a if (parts.length == 2 && "add".equals(parts[1])) { Seems like there could be a better way to configure this rather than hard-coding in the source file: code-forge/src/main/java/me/nickhanson/codeforge/web/AuthGuardFilter.java Lines 45 to 69 in daf9a2a // 1) Always allow static assets if (path.startsWith("/css/") || path.startsWith("/images/") || path.startsWith("/apidocs/") || path.equals("/favicon.ico") || path.startsWith("/favicon") || path.endsWith(".css") || path.endsWith(".js") || path.endsWith(".png") || path.endsWith(".jpg") || path.endsWith(".jpeg") || path.endsWith(".gif") || path.endsWith(".svg") || path.endsWith(".webp")) { chain.doFilter(req, resp); return; } // 2) Allow your public pages (home/about/login/error) if (path.equals("/") || path.equals("/home") || path.startsWith("/about") || path.startsWith("/logIn") || path.startsWith("/logout") || path.startsWith("/error")) { chain.doFilter(req, resp); .
Demonstrates initiative and thoughtful planning to leverage available resources (time, equipment, external expertise) and meet milestones.	Demonstrates initiative and thoughtful planning to leverage available resources; intermediate milestones were met.	Demonstrates some initiative and leverages an available resource; an intermediate milestone might have been missed slightly.	Demonstrates some planning but missed milestones.	Appears to have involved minimal planning, missed milestones or failed to reach potential due to underutilized resources.
Instructor Comments	Revision. Some milestones were late, limiting the time you had for revision after feedback.
Evidence of significant revision and incorporation of feedback.	Evidence of significant revision and incorporation of feedback that results in significant improvement.	Evidence of some revision and incorporation of feedback that results in some improvement.	Evidence of minor revision only and incorporation of feedback that results in minimal improvement.	Project appears to have undergone little to no revision or incorporation of feedback.
Instructor Comments	Met. Github commits show incorporation of feedback over time. I'm confident you learned from the process and will continue revising and improving the codebase as a portfolio piece.
Complexity	Complex	Difficult	Average	Simple
Instructor Comments	Average. This had potential for a high complexity, but based on the number of tables, missing delete in the UI, and lack of true solution validation, I'd rate the complexity as average, which is ok!
Final Presentation	Presentation is engaging, delivered with enthusiasm, relevant, comprehensive and clear. Uses aids that are effective and innovative. Demonstrates a high level of technical understanding. Manages the presentation time well. Video demonstration link in repository readme.	Presentation is well-delivered, smooth, relevant and understandable. Supporting materials are effectively used. Able to explain most technical concepts.	Audience is able to follow the presentation, but there are minor disconnects in the flow or presentation is heavily scripted. Supporting materials are used and explained in context. Able to explain some technicalities.	Presentation is difficult to follow, flow could be improved. Supporting materials are used, but not explained or in the proper context. Able to explain only a few technicalities. Presentation time is poorly managed.
Instructor Comments	Met. A thorough presentation that hit the requested items for a final review! You've delivered really engaging presentations in the past, but I felt this one was less so. Maybe my epxectations were too high ;) The parts in which you "just talked" were engaging; the parts in which it seemed like you were quickly reading a script could have been better by instead using the same consistent, personal tone. I appreciated your authentic explanation of your key learning points and challenges towards the end!

Last thing- there wasn't a good issue type available for instructor feedback (or I didn't quickly see one), so I just picked one and went with it - sort of a pain!!!

Steps to reproduce

n/a

Expected behavior

n/a

Actual behavior

n/a

Logs / stack traces

n/a

Pre-checks

• I searched existing issues.
• I can reproduce this on the latest main.
• I can work on a fix and will open a PR.

[ArchILLtect]

LOL - "Last thing- there wasn't a good issue type available for instructor feedback (or I didn't quickly see one), so I just picked one and went with it - sort of a pain!!!" --> You are sooo right! Sorry about this. I realized this when Molly created her issue and just hasn't been able to create a "general" issue template yet. My apologies--this will be added to the top of my list.

Thanks for taking the time to add this--I will address these issues as soon as possible. It has been a rocky, rough semester, but I enjoyed it overall. Thanks, Paula--have a great X-mas and happy holidays! See you next year.