Backend game logic, nearly finished with tests #133
Conversation
web_backend/src/main/java/haw/teamagochi/backend/pet/logic/gameCycle/GameCycleImpl.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public void decreaseHunger(PetEntity pet) { | ||
| if(pet == null) return; |
There was a problem hiding this comment.
There is no situation were it's ok to have a null parameter. Exception seems more appropiate
if (pet == null) {
throw new IllegalArgumentException();
}
There was a problem hiding this comment.
Mostly a style issue but if the Usecase is for pet interactions and the parameter is always a pet entity it's redunant to name the pet in the method name again. I think feed(pet) is as clear as feedPet(pet).
There was a problem hiding this comment.
I would expect increase/decreate methods to return the new value, after the increate/decrease
| /** | ||
| * Runs the Game Cycle and therefore updates the state-values of the pet's | ||
| */ | ||
| void petGameCycle(); |
There was a problem hiding this comment.
Could be named more descriptive, like doWork(), run, execute, tick
| conditions.increaseHunger(pet); | ||
| conditions.decreaseFun(pet); | ||
| conditions.decreaseCleanliness(pet); | ||
| if(randomNum.nextInt(10)==1){ //health decrease is random based | ||
| conditions.decreaseHealth(pet); | ||
| }//if | ||
| status.decreaseWellbeing(pet); | ||
| status.decreaseHappiness(pet); |
There was a problem hiding this comment.
Extracting this to its own private method improves readability
There was a problem hiding this comment.
These tests are really hard to read and to understand the intention. There is a "Given -> When -> Then" schema which is mostly used, see https://martinfowler.com/bliki/GivenWhenThen.html
- Given the current state
- When some changes, actions, etc happen
- Then I expect the following new/updated state
There was a problem hiding this comment.
I have the impression that the usecases are split in a way which makes things more complicate. All the methods in UcPetInteractionsImpl.java share mostly the same logic. But i would expect logic there. The increase/decrease methods are very complicated tho.
Also I think from a OOP perspective it would be more decoupled to have ValueObjects like Health which hold the informations. They could have properties like Integer.MAX_VALUE, eg HEALTH.MAX_VALUE and make computations like health.add(int health) which handles boundaries, etc
added the game-cycle and added corresponding uc-classes and test-->(not final and not finished)