On Thu, 3 Nov 2022 08:05:24 GMT, Ajit Ghaisas <[email protected]> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 11 commits: >> >> - Merge remote-tracking branch 'origin/master' into >> 8187145.null.selection.model >> - 8187145: added tests >> - 8187145: clear selected tree table row when setting null selection model >> - 8187145: clear selection when setting selection model >> - Merge remote-tracking branch 'origin/master' into >> 8187145.null.selection.model >> - Merge remote-tracking branch 'origin/master' into >> 8187145.null.selection.model >> - 8187145: whitespace >> - 8187145: clear focus >> - 8187145: tree table view >> - 8187145: fall through >> - ... and 1 more: https://git.openjdk.org/jfx/compare/7cb8d679...fed5dfdb > > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java > line 274: > >> 272: protected void simpleSelect(MouseButton button, int clickCount, >> boolean shortcutDown) { >> 273: final int index = getIndex(); >> 274: boolean isAlreadySelected; > > Code simplification: if `isAlreadySelected` is initialized to `false` on this > line, then we can have only `if(sm != null)` condition below instead of > if-else. it's not: `isAlreadySelected` is set on line 278 or 280. Line 274 is just a declaration. > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableCellBehaviorBase.java > line 203: > >> 201: protected void simpleSelect(MouseButton button, int clickCount, >> boolean shortcutDown) { >> 202: final TableSelectionModel<S> sm = getSelectionModel(); >> 203: boolean isAlreadySelected; > > Code simplification: if `isAlreadySelected` is initialized to `false` on this > line, then we can have only `if(sm != null)` condition below instead of > if-else. same thing: `isAlreadySelected` is set on line 206 or 210. > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java > line 178: > >> 176: return null; >> 177: } >> 178: // fall through > > There is no "fall through" here as we return from all conditions in previous > case. there is a fall through (return is inside an `if` on line 173). The original code is a bit confusing, it took me a while to see the fall through cases, that's why I added explicit comments. > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java > line 192: > >> 190: return null; >> 191: } >> 192: // fall through > > There is no "fall through" here as we return from all conditions in previous > case. same comment, there is a fall through. > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java > line 486: > >> 484: return null; >> 485: } >> 486: // fall through > > There is no "fall through" here as we return from all conditions in previous > case. same comment, there is a fall through. ------------- PR: https://git.openjdk.org/jfx/pull/876
