AlessandroCogollo / SE1-Project-2022

Software Engineering Project - Politecnico di Milano - AA2021/22
2 stars 0 forks source link

pls explain role of 2nd call to "p = r.getCurrent();" #7

Closed ingconti closed 2 years ago

ingconti commented 2 years ago

Player p = r.getCurrent(); game.playAssistant(p.getId(), 1);

    p = r.getCurrent();
    game.playAssistant(p.getId(), 2);

does game.playAssistant change player?

so you have side effects?

luca-botti commented 2 years ago

This line are used in Game test class for cycle fast through phase and checking for all possible error when calling the 5 public model function accessible from the controller and the player consequently.

p = r.getCurrent(); is used to get the current player from the class RoundHanlder that implements the finite state machine of the model.

game.playAssistant(p.getId(), 2); Has prototype playAssistant(int PlayerId, int assistantId) and change the player setting (after some error checking)modifing his current assistant to the one choose with the number and removing it from his deck. Then the method invokes the roundHandler method next() that change the phase if needed.

The possible side effects of the function are tested in the playAssistantTest method in GameTest class so in the other method is used without other checking.

Also the GameTest class, exept for the black box method not yet implemented, tests only over the functionality done in Game class, so only the check if it is possible for the player to do that determined action in this state of the model, the rest of test will be done in the other class test.

ingconti commented 2 years ago

add to javadoc relevant notes you carefully wrote.

a question remains:

the double call:

Player p = r.getCurrent(); ... p = r.getCurrent();

is need or can be skipped?

luca-botti commented 2 years ago

The call to r.getCurrent(); is needed because otherwise the playAssistant method will fail because the id of player is not the current one, we could also track internally in the method the current player but we had tested yet the roundHaldler class so we could use this method so it's easier.

ingconti commented 2 years ago

maybe method names are not self explanatory.... anyway you have a side effect that force you to call get again. correct?

luca-botti commented 2 years ago

Yes, well the "side effect" is that, if the moves is valid the method playAssistant automatically implement the changes and also call the next() method so now the pointer to player in test is no more the current one because is changed by the round handler in next(). So "side effect" because we wont that playAssistat() will change also the current player and if we don't call again the getCurrent() we can't go through all phase (we need to cycle through the phase in this way because we haven't implemented yet the "loadGame" functionality so we always start the game from the start and we need to be in some phase to test certain things). Probably in the future we will create some function to go directly in a certain phase but for now, considering that we only use it in the test, i think this is the more efficent method. Definitely anyway i will describe everything in JavaDoc

ingconti commented 2 years ago

good note. maybe better to have thought to stat implementing loadGame.. maybe less trouble...