On Fri, 15 May 2020 09:29:27 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
>> I differ on this suggestion. >> My thinking is - list access in setFocusedMenuIndex() method should have >> this check. It is not up to the caller to know >> the internal details of the method. That's the root cause of Exception. I >> added another check in >> menuBarFocusedPropertyListener because, it accesses the different list - >> container.getChildren(). In general, I feel, >> the validity check near the list usage is logical and readable as well. > > hmm .. yeah I'm aware of getContainer vs. getMenus - but they should be the > same size, shouldn't they? > > Anyway, if focusedIndex != getMenus().indexOf all users of focusedIndex have > to include a check for validity. That > might be prevented by not allowing it here in the method, in setting its > value not unconditionally to the given index > but guard it against being valid: > focusedMenuIndex = index >= getMenus().size() ? -1 : index > > Then focused is either valid or -1. Good suggestion on not setting the foucusedMenuIndex unconditionally. Also, we need to check for index < -1 : just to tighten up this method. I have added this check. ------------- PR: https://git.openjdk.java.net/jfx/pull/216
