pawaitemadisoncollege
commented
3 years ago Checkpoint 2 looks good @jvjohnson1
A few things:
-
[ ] The role table has a very specific design for tomcat authentication. You will not need a list of soldiers there, but instead a column for username. Examples are provided in the week 7 materials and in my 9/30 slack post.
-
[ ] printStacktrace statements will dump to the console. These should be replaced with log statements: https://github.com/jvjohnson1/TheFortress/blob/ac5060d9b3f0dc4d9c46e11e67c724baaf87125b/src/test/java/com/thefortress/persistence/Database.java#L49-L52
-
[ ] Time log - should be improved by adding detail about what exactly you completed. The time log should include all work for this class, not just the project.
-
[ ] The volume of hibernate debug statements going to the console might make debugging difficult. You might consider putting hibernate logs into their own file and perhaps turn logging up/down such that only Errors or Warnings are logged. The Week 4: Hibernate Demo Part 4 - Logging video demonstrates this setup.
-
[ ] src/main/java should only contain .java files. There are some bootstrap files and a .csv in the java directory (or a subdirectory of java) in your project. Any frontend files such as css, js, html, jsps, etc. should be moved into the web app directory and removed from all other locations. With regard to the .csv file, creating a dba directory at the root of the project would provide an easy to find location for this kind of file. If your java classes need access to the .csv then the resources directory would be an appropriate place for it.
-
[ ] in the same vein as above, src/test/java should contain only java files. Looks like some bootstrap, images, .js, and .css stuff snuck in there as well.
Note that test coverage wasn't listed as a requirement for the checkpoint, but it is quite low because I keyed in additional classes that are not yet tested.