Documenting my comments from today's conversation for future reference on the step function:
The current signature is
def step(config, state, p)
This requires preparing objects with correct props each time the function is used or tested.
Also it mutates the state making tests more fragile - what if someone forgets to clear the state between tests - all tests may continue working without doing the right job that may lead to the false assumption that tests are working correctly without any way to catch these errors.
Another concern - you cannot write tests based on function signature, you need to carefully look over all object props appearing inside the code and test for each one of them and the choice of names and their types is now coupled to all functions and all tests.
Actual values used by step function:
// props of config
alphaA, alphaN, alphaR, alphaS, min_breath_envelope_delta, sample_frequency
// props of state
p, Tpeak, Thigh, Vhigh, vhigh, Vlow, vlow, vhigh-state.vlow, inhaling, PEEP, RIP, RR
// other args
p
Mutation can be avoided by making this function return the dictionary of new values
and subsequently merge it into the state.
Documenting my comments from today's conversation for future reference on the
stepfunction:The current signature is
This requires preparing objects with correct props each time the function is used or tested.
Also it mutates the state making tests more fragile - what if someone forgets to clear the
statebetween tests - all tests may continue working without doing the right job that may lead to the false assumption that tests are working correctly without any way to catch these errors.Another concern - you cannot write tests based on function signature, you need to carefully look over all object props appearing inside the code and test for each one of them and the choice of names and their types is now coupled to all functions and all tests.
Actual values used by
stepfunction:Mutation can be avoided by making this function return the dictionary of new values
and subsequently merge it into the state.