Open ubamrein opened 4 years ago
Another code-smell is the fact that static
variables are kind of a hidden global variable. And those are never good, because they make it harder to modularize and test things. If for example you want to test clock-skews of different clients, you have to take care that you don't overwrite the clocks from the different clients and the backend.
In addition to that, UTCInstant
has a double-function:
The minimum change IMO is to:
private static Clock
to protected static Clock
implements Closeable
, setClock
, resetClock
and close
from UTCInstant
to UTCITest
UTCITest
only available in the testsThis would make sure that nobody calls setClock
in the main code.
For testing purposes it is neat to overwrite the clock of UTCInstant. Currently, the used clock is static, which brings its various problems with it. Also the function
setClock
overwrites the clock for all future instances, which might lead to unexpected behaviour in case of forgottenresetClock
. The latter can be prevented by using the properDisposable
feature of theUTCInstant
class.However, we should overthink the architecture and discuss the issue here, rather than in #215 since it does not pose an immediate risk and should no prevent he PR from merging (it only appears in unit-tests and integration-tests).