Check vars on execution count #5214
Conversation
| } | ||
| | ||
| private getInputExecutionCount = (cellVMs: ICellViewModel[]) : number => { | ||
| this.updateExecutionCount(cellVMs); |
There was a problem hiding this comment.
updateExecutionCount [](start = 13, length = 20)
Seems weird that this get function is updating state. Maybe do the steps separately? getInputExecutionCount didn't update any state before.
There was a problem hiding this comment.
I think then I'd call it updateCurrentExecutionCount and have it call the getInputExecutionCount and subtract 1?
In reply to: 274679371 [](ancestors = 274679371)
There was a problem hiding this comment.
Or maybe just stick this on the state instead of in a member variable?
In reply to: 274680128 [](ancestors = 274680128,274679371)
There was a problem hiding this comment.
Cause what happens if there's no input box?
In reply to: 274680321 [](ancestors = 274680321,274680128,274679371)
There was a problem hiding this comment.
Yeah, not great. How about this? The basic atomic action (get execution count) get its own function. Get input doesn't set any state and does the +1. Execution count is set just where we are going to request a new set of variables which is where we need it set. I don't believe that this should be in state as it's only used in communications it's not actually part of the react rendering code or passed down as a prop.
In reply to: 274680448 [](ancestors = 274680448,274680321,274680128,274679371)
2 participants
For #4948
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)