pawaitemadisoncollege
commented
2 months ago Hi @dwellons Thank you for your thorough review of @MNuenninghoff 's project so far. From the specificity of your comments and suggestions, it's clear you put in some time to thoroughly examine the project and you posed some thoughtful questions.
I did a quick search for the word "print" and noticed a couple files where logging should be implemented instead, the Database class and the PropertiesLoader.
In reviewing this, I noticed there isn't a story for logging out - I think I missed this in my earlier review. It should be there to be thorough.
2. Is professional and free of typos, slang, etc.
3. Fully explains the problem and the solution
4. Is understandable by the average person
- Your problem statement looks professional and I don't see any typo's.
- You do a good job explaining the terms that might be unfamiliar.
- I'm not sure if it would be helpful to define plot arc or if it is a well known part of the game.
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
- The traits are displayed in the order you mentioned in the Masters Guide. I like that you will have the ability to "re roll" to choose a new trait if you want.
- The data mentioned in the problem statement as *MVP and in the user stories is all planned for.
- Would generating a villan require different traits than the ones that a generated NPC will be given, or would you be able to create a villan using the traits you have created already?
- I think it would be cool to have the theme of the application match the game in a way.
2. There is at least one 1-to-many relationship.
3. The model represents good database design
- I think your database looks great and well structured, I was really impressed by it and how organized it looks.
- There are multiple one to many relationships.
- I think having an image associated with the traits would be a cool feature to implement in the future.
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. 7. No sensitive information is visible in the repository (secrets, passwords, etc).
- I think like you mentioned, adding the database configuration files, logs etc. to your .gitignore file is a good idea to hide passwords, etc.
- None of the classes are huge and each serves its own purpose.
- The unit testing that you have for your InteractionTraits.java class is checking for CRUD
- The only little thing that stood out to me was that some of the comments have capitalization, punctuation and some don't.
You have a great project idea and I think a lot of people are going to find this to be super useful. I also had a hard time finding suggestions, you have a solid start so far and everything you have is impressively clean and well-structured. You're ready for some JSP's and I'm excited to see how you make them, and the final product!