Project Peer Review
Reviewing your colleagues' projects
General
- G1: How creative is the project? (
0
: I've seen this on Youtube, 1
: not creative 2
: average, 3
: creative, 4
: very creative, 5
: wow!)
-
Answer:
3
Creative
-
Justification: Blackjack is a known card game, changing the rules slightly would probably have made it more original.
However, creating good background music, good graphics and sound effects deserves a bonus point in creativity.
- G2: How difficult was it to implement the project? (
0
: no challenge, 1
: very easy, 2
: easy, 3
: fair, 4
: complex, 5
: insane)
-
Answer:
3
Fair
-
Justification: Even tough the game does not have many rules, implementing some of them was probably difficult.
For example the value of aces that can be both 1 and 11, and simulating an actual deck where the same card cannot be drawn twice.
- G3: List 3 positive aspects of the project. Motivate your selection.
-
1. The game feels very polished and enjoyable. It could probably even be published as an indie game and people wouldn't notice it was a university project.
-
2. The presentation of the project, both in the
README.md
file and in the video, was very effective.
-
3. Separating the game logic from the GUI is a good way to keep the code organized.
- G4: List 3 negative aspects of the project. Motivate your selection.
-
1. There are some minor problems with the GUI. First, even though the application is running in fullscreen the Windows appbar is still visible at the bottom, second, the text in the dialog showing the game rules was outside of the box (tested with a screen resolution of 1366x768).
-
2. The code in the classes in the GUI package could have probably been structured better (some methods are very long).
-
3. The game is only playable in fullscreen mode, which is probably not necessary for such a simple game.
Programming techniques
- PT1: Evaluate how well 5 programming techniques we learned throughout the course were adopted in the project. Assign a mark from
0
to 5
to each technique, where 0
means completely incorrect usage and 5
means perfect usage.
-
1. Method overriding
-
Answer:
4
-
Justification: The usage of the technique is correct, even though it was only used to override methods in the JavaFX
Application
class and no other methods.
-
2. Serialization
-
Answer:
5
-
Justification: Keeping a log of all the player's actions is very useful for debugging.
Players can also use it to keep track of all their games.
-
3. Exception handling
-
Answer:
4
-
Justification: Used correctly when dealing with file I/O, the only detail to point out is that in the
catch
block in the LogUtil
class System.out.println(e.getMessage())
should probably be replaced by e.printStackTrace()
.
The usage with custom exceptions also doesn't seem completely correct.
-
4. Custom exceptions
-
Answer:
1
-
Justification: The only use of the
BankruptException
was inside a try/catch
that instantly catches the exception.
If the try/catch
block was removed, only leaving the code inside the catch
block, the result would be the same.
-
5. Collections
-
Answer:
5
-
Justification: Perfect usage of ArrayLists to simulate the hand of cards and the deck.
Git repository
- GR 1: How appropriate is the
.gitignore
file of the project? (0
: missing, 1
: very bad, 2
: bad 3
: average, 4
: good, 5
: very good)
-
Answer:
5
Very good
-
Justification: The
.gitignore
is properly configured to ignore IDE files, compiled class files and log files
- GR 2: How appropriate is the
README.md
file of the project? (0
: missing, 1
: very bad, 2
: bad 3
: average, 4
: good, 5
: very good)
-
Answer:
4
Good
-
Justification: The
README.md
file gives a great explanation on how to run the application and how to play the game.
It's also nice how the Presentation Section talks about the project in a very colloquial way.
The only downside is that the list of programming tequniques used does not provide much information on how and where the techniques were used.
Maven
- M1: Can you compile the project via Maven? (
0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: The project can be compiled with
mvn clean compile
without any problems
- M2: Can you clean the project via Maven? (
0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: The project can be cleaned with
mvn clean
without any problems
- M3: Can you run the project via Maven? Check
README.md
on how to run the project. (0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: The project can be run with
mvn javafx:run
as described in the README.md
file
- M4: Are the project's dependencies appropriately configured in
pom.xml
? (0
: no, 1
: some, 2
: yes)
-
Answer:
2
Yes
-
Justification: The
pom.xml
file includes all necessary dependencies
- M5: Are all Maven plugins appropriately configured in
pom.xml
? (0
: no, 1
: some, 2
: yes)
-
Answer:
2
Yes
-
Justification: The
pom.xml
file includes all necessary plugins
- M6: Does the project adopt Maven's standard directory layout? (
0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: The project follows Maven's standard directory layout
Testing
- T1: Are all tests passing? Run
mvn test
. (0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: All 9 tests pass when running
mvn test
- T2: How well do the tests cover the code? (
0
: no tests, 1
: no important method, 2
: some important methods, 3
: most important methods, 4
: all important methods, 5
: 100% test coverage)
-
Answer:
3
Some important methods
-
Justification: The test classes cover the code for some classes that handle the game's logic.
The
Deck
class is also missing in the tests.
Classes in the GUI
package also have no tests at all, some methods that are not strictly related to the user interface could use some tests.
- T3: Do the tests actually verify the expected behavior of the program? (
0
: no tests, 1
: useless tests, 2
: most don't, 3
: some do, some don't, 4
: most do, 5
: awesome tests)
-
Answer:
4
Most do
-
Justification: Most tests seem to properly verify the behaviour of some methods.
However, the two tests in the
CardTest
class seem to be redundant.
Some of the tests in the PlayerTest
class seem to verify too many methods in the same test.
- T4: How well can you understand what the tests are supposed to verify? (
0
: no tests, 1
: what is going on?, 2
: most are not understandable, 3
: some are understandable, some aren't, 4
: most are understandable, 5
: all test are understandable)
-
Answer:
4
Most are understandable
-
Justification: The tests are mostly understandable, but the name of all tests begins with 'should', even in some case where it doesn't make sense. For example
shouldAddMoney
, which tests the getMoney
method, could be called testGetMoney
, or shouldResetHand
could be called shouldHandValueBeZeroAfterReset
.
Documentation
- D1: How understandable is the Javadoc written for classes, fields and methods of the program? (
0
: no documentation, 1
: very poor, 2
: poor, 3
: average, 4
: good, 5
: awesome)
-
Answer:
5
Awesome
-
Justification: The documentation is easily understandable
- D2: How useful is the Javadoc written for classes, fields and methods of the program? (
0
: no documentation, 1
: irrelevant, 2
: little utility, 3
: average, 4
: useful, 5
: very useful)
-
Answer:
3
Average
-
Justification: The documentation for each method gives a brief explanation of what the method does.
However, some methods are fairly long, and a one-line explanation doesn't seem to be enough in some cases.
- D3: Can you generate documentation files for this project? Run
mvn javadoc:javadoc
. (0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: Javadoc is generated correctly when running
mvn javadoc:javadoc
- D4: How adequate are the non-javadoc comments written throughout the code? (
0
: mostly inadequate, 1
: average, 2
: mostly adequate, 3
: awesome)
-
Answer:
2
Mostly adequate
-
Justification: Non-javadoc comments are used to separate parts of some long methods.
Code quality
- Q1: Is the code style adopted throughout the project consistent? Consider how whitespace is represented (spaces or tabs), tab size, naming conventions for classes, methods and variables, indentation, braces usage, line width. See Google Java Style Guide as an example of code style guidelines. (
0
: no, 1
: yes, but..., 2
: yes)
-
Answer:
2
Yes
-
Justification: The code uses a consistent style, even between classes written by different authors.
- Q2: How would you rate the project in terms of code duplication? (
0
: a lot of duplication, 1
: some duplication, 2
: barely any duplication, 3
: no code duplication / only justifiable duplication)
-
Answer:
3
No code duplication / Only justifiable duplication
-
Justification: There doesn't seem to be any duplication at all.
- Q3: How easy it is to understand how the program works by looking at the source code? (
0
: mostly hard to understand, 1
: some fragments are hard to follow, 2
: not hard, but not easy, 3
: easy to understand)
-
Answer:
2
Not hard, but not easy
-
Justification: The code that handles the game logic is easy to understand, but the classes that handle the GUI have some long methods that could be split into smaller sections to make them easier to read.
- Q4: Is any section of the program excessively inefficient? (
0
: mostly hard to understand, 1
: some fragments are hard to follow, 2
: not hard, but not easy, 3
: easy to understand)
-
Answer:
3
-
Justification: The program does not seem to have any excessive inefficiency
- Q5: Does the program crash unexpectedly (e.g. by an uncaught exception)? (
0
: all the time, 1
: rarely, 2
: it happened once, 3
: never)
-
Answer:
3
Never
-
Justification: No crashes were experienced after playing the game for some time.