2014-11-02 12:58:01
- Added more tests to ProductionServiceSpec and cleaned up the code.
- Use Google Guava Ints/Longs.tryParse() to make code more elegant.
- Use new OptionalResult class instead of new RuntimeException to report problems.
(The ProductionController still uses the old problem reporting method via Exception, but is now ready for further refactoring)
- Add OptionalResult<T> class
This helps reducing the places where problems are communicated via RuntimeException
and also reduces cases where nullable objects are returned and not properly handled.
The old pattern in Little Goblin is:
1. if(problem){throw new RuntimeException(errorMessage))
2. catch RuntimeException e, render error message from e.getMessage (this breaks with unexpected exceptions lacking a proper messageId...)
The new pattern should be:
1. for each problem: add error to OptionalResult
2. return OptionalResult and handle valid/invalid case from there
The old pattern mixes serious exceptions with common user errors (for example, using a too-short password),
it often uses expensive Exception objects (with complete StackTraces) and can only ever communicate one problem.
You can work around the expensive objects problem by using static exceptions, but it's still ugly.
- Remove a hasMany connection, which makes it easier to unit test.
- Use Java 8
- Changed formatting; add custom toString() to several classes to to improve debugging and test output.
- Upgrade to Grails 2.4.4 - Closes #97
- Add Google's Guava library.
- This lib provides many useful constructs, for example Longs.tryParse(str) which can parse Strings and returns null instead of throwing NumberFormatException in case of invalid input.
- Started work on ProductionServiceSpec.
- Refactored ConstraintUnitSpec so it's no longer a parent class for unit tests.
The idea of a class which could provide all kinds of mocked items for tests was good, but failed to take implementation details into account. Different unit tests would need specialized Item or Product instances and this complexity would care over to the providing class. Also unit tests with TestFor annotation would not necessarily want those classes to be Mock'ed. Current test writing strategy will be to make the test classes as independent as possible. A little bit more redundant, a lot less confusing/complex.
- Update Tomcat plugin to version 8.
- Made Creature class abstract as there is no use case for instantiating Creature objects - LG always uses sub classes.
Also I think this might make ORM easier.
- In Item class changed field owner from Creature to PlayerCharacter.
Reason: I was having a hard time using creating a unit test that would work with the extending class PlayerCharacter with this field.
Then I changed Creature to be an abstract class as there won't be any Creature objects instantiated directly anyway.
Now it looks like the ProductionServiceSpec will run without problems.
Only drawback: Mobs which extends Creature cannot own Items now. But then was already a need to write new code for Mobs using Items.
- Fix #93; computeMaxProduction to return correct amount.