Open bryanmk opened 1 year ago
bryanmk
commented
1 year ago I just read the review you left for me aaand just to reiterate your own thoughts back to you --
You may have some copyright type things to consider as well when you start pulling data/images from official Star Trek places. I feel this is a smart one to have written down somewhere.
tmirkes
commented
1 year ago Absolutely. I've been looking at some templates that are sort of a "Stare Track" approach; reminiscent of the IP but not a direct use of the iconography. But yeah, I'll have to be careful not to lean too heavily on the identity materials...
pawaitemadisoncollege
commented
1 year ago Hi @bryanmk! Thank you for your thoughtful and thorough review.
I did notice a couple sneaky print stack trace statements that should be replaced with log4j - thank you for leaving something for me to find ;) 
Design/Code Review 1
Project: Trek Roulette
Developer: Tim Mirkes
Reviewer: Matt Bryan
2. Is professional and free of typos, slang, etc.
3. Fully explains the problem and the solution
4. Is understandable by the average person
2. Much like my app, you have a very specific target audience. It may be helpful to explain why a non-Trekkie would want to use it.
3. I've been trying to think of a second suggestion for your problem statement for the last day and honestly I'm coming up with nothing. I fully understand what the app is and why one would use it, and I don't even consider myself the target audience. It's a good problem statement.
2. The order in which values are displayed are logical and easy to understand/use
3. The order in which the form fields entered are logical and easy to understand/use
4. All data discussed/documented (problem statement, flow, db design, etc.) is represented on the screens
2. It's more of a feature than anything inherently wrong, (and I'm not a Trekkie), but I think it would be very nice to be able to generate an episode by season or series in addition to getting a single episode out of everything ever.
3. It's certainly easy to have everything all on the index page, but I think one way you could go with this is to have different pages for each series and have their own generators there (if you follow my previous feature request). Just a thought as to how to possibly build out the project more.
2. There is at least one 1-to-many relationship.
3. The model represents good database design
2. A con for my previous pro would be that breaking everything up into so many objects certainly could create complications in making sure everything is talking to everything else correctly.
3. I think it's a good idea to figure out how you're going to access all of the data that you need and how. API's will be an option, then you mentioned just doing a single pull of the info and possibly having a script run every now and again to make sure you have every episode. I mention this because it depending how you approach this it may requre some alterations to your existing structure.
2. a .gitignore file for IntelliJ Java projects has been implemented
3. There is not any redundant or copy/paste code in the JSPs or classes
4. Classes are appropriately-sized (no monster classes)
Property files are used appropriately: no hard-coded values
5. Logging statements are used rather than System.out.println and printStackTrace.
6. There are appropriate unit tests/code coverage.
2. Just like me, I think you've got to get better at logging. There isn't much of that happening yet in this project and I'm guessing it will be useful to have handy at some point.
3. You've got the GenericDAO going strong, but I think you'd benefit from having the other classes represented in addition to the that in order to run more specific tests and make sure every aspect of your app is functioning the way you'd like it to be.