Bug 132947 [DataBinding] small code style problems in AbstractObservable
I read new code for JFace data bindings and found couple code style problems in AbstractObservable. Nothing special, but looks not ideal, as it should be Eclipse code. :-) 1. In "addChangeListener". Collection listenerList; if (changeListeners instanceof IChangeListener) { IChangeListener l = (IChangeListener) changeListeners; listenerList = new ArrayList(); listenerList.add(l); } else { listenerList = (Collection) changeListeners; } if (listenerList.size() > 16) { HashSet listenerSet = new HashSet(); listenerSet.addAll(listenerList); changeListeners = listenerList; } Here listenerList declared as collection and initialized with ArrayList. But couple lines below listenerSet declared not as Collection or even Set, but directly as HashSet. 2. In "removeChangeListener". Collection listenerList = (Collection) changeListeners; listenerList.remove(listener); if (listenerList.size() == 0) { changeListeners = null; if (!hasListeners()) { lastListenerRemoved(); } } I think that it is better use "listenerList.isEmpty()" instead of "listenerList.size() == 0". Here I have also two sub-questions: 2.1. may be check that remove() returns "true"? In theory it is possible that collection does not contains passed listener (because of bug in user code?), so rest of code is not required. 2.2. I am not sure, but may be it is good to check for single element is listenerList and rollback to direct listener as changeListeners? Well, most probably no, but just idea...
bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/internal/viewers/StructuredViewerObservableCollectionWithLabels.java
}
		}
		if (adds.size() > 0) {
			fireListChange(Diffs.createListDiff((ListDiffEntry[]) listAdds
					.toArray(new ListDiffEntry[listAdds.size()])));
			fireSetChange(Diffs.createSetDiff(adds, Collections.EMPTY_SET));
			
			return true;
		}
		return false;
	}
	public void dispose() {
		super.dispose();
		wrappedList.clear();
		structuredViewer = null;
		contentProvider = null;
	}
	protected abstract void addToViewer(Object element);
	protected abstract void addToViewer(Object[] elements);
	protected abstract void addToViewer(int index, Object element);
	protected abstract void removeFromViewer(Object element);
	protected abstract void removeFromViewer(Object[] elements);
	/**
	 * @return Returns the structuredViewer.
	 */
	public StructuredViewer getViewer() {
		return structuredViewer;
	}
}