1st Milestone Evaluation - Creatures hunting, group 6

1st-milestone-group-6.md

1st Milestone Evaluation

• Review author: Richard Trebichavský (410115)
• Project: Creatures hunting, group 6
• Team leader: Bc. Marek Turis (422805)

Project Requirements

• create a project in a Github repository that is publicly accessible (for read) choose a short and descriptive name. Create some project wiki to publish other information for your project.
  • https://github.com/mayoturis/PA165_CreaturesHunting
  • Fulfilled
• on the project wiki there will be a project description, a use case diagram and a class diagram for entity classes.
  • https://github.com/mayoturis/PA165_CreaturesHunting/wiki
  • Fulfilled
• There will be at least two user roles in the use case diagram.
  • User, Administrator
  • Fulfilled
• Associations between entities will be present in the class diagram.
  • Fulfilled
• create 4 entity classes for your project.
  • Weapon, Monster, Area, User
  • Fulfilled
• create a DAO layer interface (with proper javadoc).
  • The update() method is missing on all entities except Weapon. This violates requirement from the assignment “Administrator of the system should be able to perform CRUD operations on all entities.”
  • Partially fulfilled
• create the JPA implementation of the DAO classes (CRUD operations are enough for the first checkpoint).
  • Fulfilled
• create unit tests for DAO classes (use in-memory database).
  • Nicely tested, creation with incorrect parametres.
  • I find AreaDaoTest::createBadInputTest() method name confusing and I would use createIncompleteInputTest(). Current name invokes that "BadArea" is invalid name for Area, which is not true.
  • Fulfilled
• every team member should work with different entities on different parts of the project (e.g. member 1 will create implementation of DAO for entity A, but member 2 will create unit test for entity A). In every class there will be javadoc @author with the name of the class author.
  • Fulfilled
• Also you must commit into Git only the changes that you made yourself. If you commit on behalf of somebody else this will not be regarded as his work!
  • Fulfilled
• the project will be built using maven, and make sure you have all dependencies correctly configured, so it is possible to build it using just the command line.
  • Installation of dependencies via mvn clean install works correctly,
  • However, invoking mvn clean test will not run any tests.
  • Partially fulfilled

Evaluation Checklist

You can assign maximum 10 points to the project you are reviewing.

• -3 points if it is not possible to compile the project using “mvn clean install”. You must either make this work or ask the team to fix this ASAP because it’s hard to check the code without this.
  • I would subtract 0.5 point because tests could not be run from command line. However, this requirement was not explicitly mentioned in assignment and the team members were probably not aware about it, but in practice it is very important that this is configured and works correctly.
• -1 point for each minor occurrence of a team member not contributing enough.
• -1 points for incorrectly implementing equals/hash code.
• -1 points for every minor occurrence of missing methods/tests (e.g. missing important method on a DAO object or a missing test for that method).
  • Missing update() methods on all DAO's except Weapon.
• If a team member was not contributing at all, or very little, you should explicitly say this in your evaluation report. Tutor will follow up on this information.

Notes

• You are using both jUnit and TestNG, I don't understand why in such early stage of the project. You should keep just one, I suggest TestNG. See https://www.mkyong.com/unittest/junit-4-vs-testng-comparison/
• All tests are testing an interface, but they should test the implementation.
• In cz.muni.fi.pa165.dao.AreaDaoTest, properties area, area2 and area3 should be private.
• Entities should not expose a public constructor that allows to create invalid entity instance. This way, user could easily create an entity that is not valid and will receive error at point when entity is persisted to database, which is too late. Therefore, every entity should have a protected empty constructor (required by hibernate) and public constructor with all required fields, e.g.

protected User() {
	// required by Hibernate	
}

public User(String password, String email, String name) {
	setPassword(password);
	setEmail(email);
	setName(name);
}

• Unused imports in
  • cz.muni.fi.pa165.ApplicationContextConfiguration
  • cz.muni.fi.pa165.entities.Area

Conclusion

I propose to evaluate milestone with 8.5/10 points, because of missing update() methods and impossibility to run tests form CLI.