Skip to content

Commit

Permalink
[Modernize Code] clean-ups in AbstractEditPartViewer (#287)
Browse files Browse the repository at this point in the history
  • Loading branch information
azoitl authored Dec 8, 2023
1 parent e049527 commit a6b6313
Showing 1 changed file with 10 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.eclipse.gef.EditPart;
import org.eclipse.gef.EditPartFactory;
import org.eclipse.gef.EditPartViewer;
import org.eclipse.gef.GraphicalEditPart;
import org.eclipse.gef.KeyHandler;
import org.eclipse.gef.RootEditPart;
import org.eclipse.gef.SelectionManager;
Expand Down Expand Up @@ -82,7 +83,7 @@ public abstract class AbstractEditPartViewer implements EditPartViewer {
* @deprecated
*/
@Deprecated
protected List selectionListeners = new ArrayList(1);
protected List<ISelectionChangedListener> selectionListeners = new ArrayList<>(1);

/**
* The editpart specifically set to have focus. Note that if this value is
Expand All @@ -96,9 +97,9 @@ public abstract class AbstractEditPartViewer implements EditPartViewer {
protected EditPart focusPart;

private EditPartFactory factory;
private final Map mapIDToEditPart = new HashMap();
private final Map mapVisualToEditPart = new HashMap();
private Map properties;
private final Map<Object, EditPart> mapIDToEditPart = new HashMap<>();
private final Map<IFigure, GraphicalEditPart> mapVisualToEditPart = new HashMap<>();
private Map<String, Object> properties;
private Control control;
private ResourceManager resources;
private EditDomain domain;
Expand Down Expand Up @@ -254,11 +255,8 @@ public final EditPart findObjectAtExcluding(Point pt, Collection<IFigure> exclud
* Fires selection changed to the registered listeners at the time called.
*/
protected void fireSelectionChanged() {
Object listeners[] = selectionListeners.toArray();
SelectionChangedEvent event = new SelectionChangedEvent(this, getSelection());
for (Object listener : listeners) {
((ISelectionChangedListener) listener).selectionChanged(event);
}
selectionListeners.forEach(lst -> lst.selectionChanged(event));

This comment has been minimized.

Copy link
@iloveeclipse

iloveeclipse Jan 16, 2025

Contributor

This change introduced regression. Selection listeners that were removing themselves after selection is changed cause now exception

java.util.ConcurrentModificationException
 at java.base/java.util.ArrayList.forEach(ArrayList.java:1598)
 at org.eclipse.gef.ui.parts.AbstractEditPartViewer.fireSelectionChanged(AbstractEditPartViewer.java:258)
 at org.eclipse.gef.SelectionManager.fireSelectionChanged(SelectionManager.java:152)
 at org.eclipse.gef.SelectionManager.deselect(SelectionManager.java:127)

This comment has been minimized.

Copy link
@merks

merks Jan 16, 2025

Contributor

One should always be concerned that a list that hold listeners might change because of the actions of the listeners. (One might also consider that more modern patterns have modern performance, i.e., not better performance, and are less easy to debug.)

}

/**
Expand Down Expand Up @@ -497,12 +495,8 @@ protected void init() {
}

private void primDeselectAll() {
EditPart part;
List list = primGetSelectedEditParts();
for (Object element : list) {
part = (EditPart) element;
part.setSelected(EditPart.SELECTED_NONE);
}
List<? extends EditPart> list = primGetSelectedEditParts();
list.forEach(part -> part.setSelected(EditPart.SELECTED_NONE));
list.clear();
}

Expand All @@ -511,7 +505,7 @@ private void primDeselectAll() {
*
* @return the internal list of selected editparts
*/
protected List primGetSelectedEditParts() {
protected List<? extends EditPart> primGetSelectedEditParts() {
return selection;
}

Expand Down Expand Up @@ -766,7 +760,7 @@ public void setKeyHandler(KeyHandler handler) {
@Override
public void setProperty(String key, Object value) {
if (properties == null) {
properties = new HashMap();
properties = new HashMap<>();
}
Object old;
if (value == null) {
Expand Down

0 comments on commit a6b6313

Please sign in to comment.