-
Notifications
You must be signed in to change notification settings - Fork 4
Description
Hi Kir,
I'm moving the discussion here so that it is not lost after a push. I believe that I have understood your proposal. Please find my discussion below. I'm trying to be clear, concise and kind with my writing. Please don't take any offense, because no offense is intended. :)
Kir wrote:
Based on you change, I have thought about those issues in stub for a while. And I came up a quite new design for handling them. Your design is like a Finite-state machine, and we have 2 states so far. However, it might be hard to maintain in the future without proper document.(which state have what function? what is the next state?)
Let us use the term One-Object-Per-Condition (OOPC) to describe the proposed solution in the pull request.
Let us use the term NFSM for the New Finite State Machine solution you are proposing. Reproduced below:
stub_function_fsm_table = {
"onCall": {"returns", "throws"},
"withArgs": {"onCall", "returns", "throws"}
}
CURRENT_STATE = ""
def fsm_control(func, *args, **kwargs):
def inner_fsm_control():
if CURRENT_STATE and func.__name__ in stub_function_fsm_table[CURRENT_STATE]:
CURRENT_STATE = func.__name__
return func(*args, **kwargs)
else:
raise Exception("not allowed")
return inner_fsm_control
@fsm_control
def onCall(...):
...The OOPC uses one object to hold one condition. In OOPC, if the user does not capture a newly created object by saving the object in a variable, or calling returns or throws, then the state is reset. E.g.
stub = sinon.stub()
stub.withArgs(3) # this condition is lost
stub.returns(5) # line 3, state is reset
assert stub() == 5OOPC resets the state in line 3, so the stub will always return 5. With NFSM, if you don't return a new object in the call to withArgs, how would you reset the state in this way? I don't believe that NFSM can meet this requirement.
There are two benefit from this design.
- Easy to maintain without document + Extendable + High performance O(1).
- Lower memory consumption (without creating new memory for new object but reference object itself)
I agree that the maintainability of NFSM would be better, since it clearly lays out the states and possible transitions. However, if it doesn't meet the requirements then a more complex solution like OOPC is required.
Regarding performance: unless 10,000 or more operations are being executed, performance is not relevant. In the case of OOPC, users will never be creating more than 20 or 30 objects. Hence, I don't see any performance issue here, in terms of time or memory consumption.
What do you think?
I hope that helps,
--Jonathan