On Thu, 22 Apr 2021 10:40:15 GMT, Jeanette Winzenburg <[email protected]> wrote:
>> I see the updateEditingIndex is used as a hacky flag as the comment says, >> and `table.edit(-1, null)` is called conditionally. >> 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. >> 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. > > 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 :) ------------- PR: https://git.openjdk.java.net/jfx/pull/473
