On Fri, 23 Apr 2021 11:40:00 GMT, Johan Vos <[email protected]> wrote:

>> thanks for the explanation and spelling it out to me :)
>> 
>>> But my question is about the pre- and postconditions. What would happen if 
>>> a subclass overrides `cancelEdit` and throws an exception, so that 
>>> `updateEditingIndex = true` is not called after the `cancelEdit`? In that 
>>> case, `updateEditingIndex` stays false.
>> 
>> talking about pre/postCondiditions: wouldn't an override of cancelEdit (and 
>> the other editing life-cycle methods) that's throwing an exception violate 
>> its contract? Don't see any constraints in its spec at Cell, so I would say 
>> it must do it's very best not to throw anything, at least not intentionally. 
>> And if it happens somewhere along the chain, f.i.  an edit handler going 
>> wild, then it would be the responsibility of that handler to behave (aka: 
>> usage error except for the little bugs in the editEvents .. ). 
>> 
>>> I'm not saying this is an issue, just wondering about this as this flag 
>>> does an important thing -- and I do know this was in the code before the 
>>> PR, so the PR is definitely an improvement, I'm just thinking that this 
>>> might be an opportunity to deal with the precondition that 
>>> updateEditingIndex _should_ be true when entering the method and false when 
>>> leaving the method.
>> 
>> hmm .. again not quite sure I understand: shouldn't updateEditingIndex be 
>> true both on entering and leaving doCancelEdit?. Anyway, if there's 
>> something going wrong in cancelEdit, the cell is in an arbitrary state: 
>> could still be editing (if the wrong-going happened before calling 
>> cell.cancelEdit) or not, could have fired the cancel event or not, could 
>> have updated its visuals or not .. there's not much we can do to recover 
>> from it. When going from this undeterminate state into the next editing 
>> cycle the outcome is .. unpredictable. 
>> 
>> All that said: we could wrap the flag un-/setting into a try-finally block - 
>> then at least the whacky flag is in a specific state after all hell broke 
>> loose ..
>
> I agree a try-finally block here (setting the flag in a specific state) might 
> be useful -- I prefer to keep unpredictable states for quantum computing :)

done for Tree/TableCell: 

- wrapped into try-finally block with resetting updateEditingIndex flag to true 
in finally
- added test to Tree/TableCellTest that fails before and passes after the change

now the flag is more predictable than the behavior of extending classes' 
cancelEdit implemenations (and states in quantum computing - probably, your 
book is still on my reading list :)

-------------

PR: https://git.openjdk.java.net/jfx/pull/473

Reply via email to