#2363 Calculate player's recovery#2450
Conversation
# Conflicts: # src/main/java/core/gui/comp/table/HOTableModel.kt
# Conflicts: # src/main/resources/release_notes.md
| return whenSlightlyInjured; | ||
| } | ||
|
|
||
| public boolean getIsInvalid() { |
| * Date is null if player is healthy | ||
| */ | ||
| private HODateTime whenHealthy; | ||
| /** |
| return getInjury().getTypeOfEstimate(); | ||
| } | ||
|
|
||
| public Boolean isInvalid() { |
| private static double calcMedicianFactor(int doctorLevel) { | ||
| double x1Factor = 0.2124; | ||
| double x0Factor = 1; | ||
| return (x1Factor * (double) doctorLevel + x0Factor) / (x0Factor + 5 * x1Factor); |
| private static double calcMedicianFactor(int doctorLevel) { | ||
| double x1Factor = 0.2124; | ||
| double x0Factor = 1; | ||
| return (x1Factor * (double) doctorLevel + x0Factor) / (x0Factor + 5 * x1Factor); |
There was a problem hiding this comment.
5 should be a constant instead of magic number.
| /** | ||
| * Health calculation based on Schum formula (<a href="https://www82.hattrick.org/Forum/Read.aspx?t=17404127&n=6&v=0&mr=0">...</a>) | ||
| */ | ||
| public class Injury { |
There was a problem hiding this comment.
Class should be package private if the sole constructor is also package private.
| */ | ||
| private boolean isInvalid = false; | ||
|
|
||
| public enum TypeOfEstimate { |
There was a problem hiding this comment.
When Injury is only the calculation and all values are delegated to Getters in Player then this enum should be a general public enum outside that implementation class. Maybe InjuryEstimationType
| public enum TypeOfEstimate { | ||
| REALISTIC_ESTIMATE, | ||
| OPTIMISTIC_ESTIMATE, | ||
| PESSIMISTIC_ESTIMATE, |
There was a problem hiding this comment.
Not needed I think - if not injured, the whenHealthy date is null, meaning that it is realistic that we don't need any recovery.
| PESSIMISTIC_ESTIMATE, | ||
| } | ||
|
|
||
| private TypeOfEstimate typeOfEstimate = TypeOfEstimate.REALISTIC_ESTIMATE; |
There was a problem hiding this comment.
If the player isn’t injured, which is hopefully the most likely situation for most players, no calculations are done, which means it’s realistic that no recovery is needed.
| return regainerFactor * calcAgeFactor(player.getAgeAtDate(dateTime).toDouble()) * calcMedicianFactor(doctorLevel); | ||
| } | ||
|
|
||
| private void calculateRecovery(Player player) { |
There was a problem hiding this comment.
Method is very long: Could it be separated in parts of different sub functions?
| /** | ||
| * True if the player is an invalid (no recovery possible) | ||
| */ | ||
| private boolean isInvalid = false; |
There was a problem hiding this comment.
Suggestion: chronicInvalid as a name because invalid is very general and could be a state of the class instance itself.
# Conflicts: # src/main/resources/release_notes.md
| } | ||
| public List<FactorObject> getFaktorenFromDB() { | ||
| return load(FactorObject.class); | ||
| // if (!factors.isEmpty()) { |
There was a problem hiding this comment.
Please delete code that is not used anymore.
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertIsNot | ||
|
|
||
| class InjuryTests { |
There was a problem hiding this comment.
All tests should be named with the name of the class with the postfix "Test" in singular.
|
|
||
| class InjuryTests { | ||
| @Test | ||
| fun testInjury() { |
There was a problem hiding this comment.
That is a test but it should cover the cases that could happen (each at least for one possible result).
So multiple tests or if possible better a parameterized test.
I can offer to write tests for that class if you wish. Then you have a template.
There was a problem hiding this comment.
this is in progress - I will push the draft, which doesn't run successfully until now.
It is still a lot to do because the injury calculation requires a lot of data from the ho model, which will be provided by the new implemented test persistance manager.
| @Test | ||
| fun testInjury() { | ||
|
|
||
| val hoAdmin = HOVerwaltung.instance() |
There was a problem hiding this comment.
Is the instance of HOVerwaltung really necessary?
There was a problem hiding this comment.
The injury calculation uses HOVerwaltung.instance.
2 participants
changes proposed in this pull request:
src/main/resources/release_notes.md...