gemini-hlsw/ocs — pull request #112 by swalker2m (2014-11-24 14:02:54+00:00)
Title: REL-2127: target selection grief

This is one of those ridiculous problems that will likely bring Rob to tears again. I too am gnashing my teeth and rending my garments. The PR addresses a bug with target selection in the telescope position editor. Here is a “solution” but I can’t stomach it and __I need help coming up with better alternatives__, if there are any. Fixing a bug in the OT is a little bit [like this](http://imgur.com/mWPefaQ).

In the target list editor you can add new guide stars and so-called “user” targets for blind offsets. Ordinarily when you add a new target the editor selects the target in the table and a selection listener handles switching the target position editor controls below to display the newly created target. This happens whether you add the target with the green + button or via the TPE by clicking on a star.

The root cause of the bug is that target selection is kept in the target component node’s “transient user data” as a `java.lang.Integer` object. The integer corresponds to the target’s position in the target list. That works except when a new target is added *that takes the place of an existing target*. For example, start with a base position and a PWFS target in a GMOS observation. If the PWFS target is selected, its index is 1 and 1 is stored as the selected target index. Now we add a GMOS OIWFS guide star which sorts ahead of the PWFS guide star taking the index position 1 and leaving the PWFS guide star at index 2.

Because the index remains at 1, the selection of the new GMOS OIWFS target is ignored because the `PropertyChangeSupport` employed by `MemAbstractBase` to fire change events will ignore an update in which the new and old value of the property is the same according to `equals`. Having added a new GMOS OIWFS target you’re left looking at the PWFS guide star. Worse still manually selecting the new GMOS OIWFS target to edit it doesn’t work either because (again) the selected index doesn’t change.

Ideally we’d just store the selected target itself instead of its index in the target list. Unfortunately that won’t work because the target is mutable and multiple defensive copies are made yet the position editor and telescope pos list must share the concept of the selected target.

So this “solution” option is to store a different integer wrapper in which `equals` and `hashCode` are distinct for each new instance regardless of the value they contain. In that way selecting position 1 a second time is treated as a selection update and everything “works”. Of course it isn’t quite that easy as there is one place in the target editor where we were relying on the existing behavior so that had to be addressed with the awesome `ignoreEvents` flag. Grief all around.