Skip to content

Refactor/new object diagram fixes#125

Open
ahmedkha79 wants to merge 22 commits intouseocl:masterfrom
ahmedkha79:refactor/NewObjectDiagram-fixes
Open

Refactor/new object diagram fixes#125
ahmedkha79 wants to merge 22 commits intouseocl:masterfrom
ahmedkha79:refactor/NewObjectDiagram-fixes

Conversation

@ahmedkha79
Copy link

Zweck
Refactorings und Warnungsbereinigungen rund um NewObjectDiagram und angrenzende Stellen. Ziel: Kompilierbarkeit wiederherstellen, Null-Safety verbessern, Analyse-Warnungen reduzieren und Lesbarkeit erhöhen. Keine bewusst eingeführten API-Breaking-Änderungen.

Zusammenfassung der Änderungen (thematisch geordnet)

  1. Map-Safety & Null-Checks
  • Ersetzte riskante Map.containsKey + Map.get Muster durch sicherere Varianten (null-checks, computeIfPresent).
  • Behobene fehlerhafte Map-Zugriffe, die zu Compile- oder Laufzeitfehlern führten.
  1. Listener & Event-Handling
  • Entferntes unbenutztes Feld/innere Klasse ShowObjectPropertiesViewMouseListener.
  • Vereinfachte MouseListener-Handler (pattern matching instanceof wo sinnvoll).
  • Handler und Actions dort, wo sinnvoll, als final markiert.
  1. Data & API
  • Vereinheitlichte Handhabung sichtbarer/verborgener Daten via ObjectDiagramData-Accessoren (getVisibleData/getHiddenData).
  • Ergänzte/implementierte Methoden, die fehlten oder zu Kompilationsfehlern führten (z. B. getLinkByValue, getHiddenNodes, getDefaultLayoutFileSuffix).
  1. Readability & Helpers
  • createAction-Helper hinzugefügt, um wiederkehrende Action/Listener-Boilerplate zu reduzieren.
  • Modernisierung: Lambdas, Method-Refs, Pattern Matching instanceof, computeIfAbsent/computeIfPresent.
  • Kleinere Kommentare und Lesbarkeitsverbesserungen.
  1. Analyzer & Warnings
  • Punktuelle @SuppressWarnings("unused"), wo API-Overloads für Kompatibilität bestehen.
  • Diverse Typos/JavaDoc-Ergänzungen, Reduktion redundanter isInstance() Checks und Entfernung von doppelten containsKey/get-Kombinationen.

Betroffene Hauptdateien

  • use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
  • use-gui/src/main/java/org/tzi/use/gui/views/diagrams/elements/edges/AssociationOrLinkPartEdge.java
  • use-gui/src/main/java/module-info.java
  • use-core/src/main/java/org/tzi/use/util/Debug.java

Risiken & Tests

  • Low-risk-Refactorings; trotzdem empfiehlt sich manuelles Testen der GUI-Flows (z. B. Restore/Delete in Object-Diagrammen) und ein vollständiger Build (mvn -DskipTests=false install) vor dem Merge.

Anmerkung

  • Diese PR konzentriert sich auf lokale Verbesserungen und Warnungsreduktion; größere Umstrukturierungen wurden bewusst vermieden, um die Review- und Merge-Komplexität gering zu halten.

ahmed and others added 11 commits January 29, 2026 01:23
- Automatic Deletion of all unnecessary imports in use-gui with Copilot
…ty and consistency

- Use diamond operator (<>) to reduce generic verbosity
- Replace `size() > 0` checks with `isEmpty()` for clearer intent
- Convert simple anonymous listeners to lambdas to reduce boilerplate
- Remove redundant null checks before `instanceof`
- Simplify ternary negation and other small readability fixes
- Prepare code for follow-up refactors (extracting data structures, controllers)

No behavioral changes intended; this is a mechanical cleanup to reduce cognitive load and prepare the file for safer, larger refactors.
…or clarity improvements

- Fix broken Map access (use getLinkObjectToNodeEdge().get(link)) which caused compile errors and broken restore/delete flows.
- Add null checks before invoking methods on edges during restore and delete to avoid NPEs when layout data is missing.
- Change javaFxCall from boxed Boolean to primitive boolean and update setter for NPE safety and clarity.
- Correct copyTo Javadoc param description.
These are small, low-risk fixes to restore compileability and improve robustness and readability. No behavioral change expected.
Co-authored-by: automated-refactor-agent <refactor@example.com>
…e initializers, use computeIfAbsent, lambdas & method refs, improve drop logging, minor readability fixes
…accesses, use List.sort, use fLog.println, lambdas and small readability fixes
…n: - Map-safety & Null-Checks - Ersetzte unsichere Map.get()-Aufrufe durch computeIfPresent / null-checks. - Entfernte doppelte containsKey/get-Kombinationen. - Listener & Event-Handling - Entfernt unbenutztes Feld/innere Klasse ShowObjectPropertiesViewMouseListener. - Vereinfacht MouseListener mit Pattern Matching (instanceof). - Data & API - Vereinheitlichte Handhabung sichtbarer/verborgener Daten via ObjectDiagramData-Accessoren. - getVisibleData/getHiddenData abgestimmt. - getDefaultLayoutFileSuffix gesetzt. - Readability & Helpers - createAction Helper eingeführt, Boilerplate reduziert. - getLinkByValue implementiert (Restore/layouter Hilfsmethode). - Analyzer & Warnings - @SuppressWarnings(unused) wo sinnvoll (z.B. isHidden Persist-Helper Version).   - Weitere Konsistenz- und Null-Sicherheits-Fixes. Keine API-Breaking Änderungen; low-risk Refactorings zur Verbesserung der Lesbarkeit und Analysehygiene.
…n: - Map-safety & Null-Checks - Ersetzte unsichere Map.get()-Aufrufe durch computeIfPresent / null-checks. - Entfernte doppelte containsKey/get-Kombinationen. - Listener & Event-Handling - Entfernt unbenutztes Feld/innere Klasse ShowObjectPropertiesViewMouseListener. - Vereinfacht MouseListener mit Pattern Matching (instanceof). - Data & API - Vereinheitlichte Handhabung sichtbarer/verborgener Daten via ObjectDiagramData-Accessoren. - getVisibleData/getHiddenData abgestimmt. - getDefaultLayoutFileSuffix gesetzt. - Readability & Helpers - createAction Helper eingeführt, Boilerplate reduziert. - getLinkByValue implementiert (Restore/layouter Hilfsmethode). - Analyzer & Warnings - @SuppressWarnings(unused) wo sinnvoll (z.B. isHidden Persist-Helper Version).   - Weitere Konsistenz- und Null-Sicherheits-Fixes. Keine API-Breaking Änderungen; low-risk Refactorings zur Verbesserung der Lesbarkeit und Analysehygiene.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs refactorings and warning cleanups around NewObjectDiagram and related classes. The main goal is to restore compilability, improve null-safety, reduce analyzer warnings, and increase readability without intentionally introducing API-breaking changes.

Changes:

  • Improved encapsulation of ObjectDiagramData by making fields private and adding public accessor methods
  • Enhanced map-safety with proper null-checks and use of computeIfPresent/computeIfAbsent
  • Modernized code with lambdas, method references, pattern matching instanceof, and diamond operators
  • Refactored persistence/restore logic into smaller helper methods for better maintainability
  • Fixed spelling errors (Derrived→Derived, Compositon→Composition, Reflexiv→Reflexive, assoziation→association)
  • Changed "Grey" to "Gray" in UI labels for American English consistency
  • Added missing method implementations (getLinkByValue, getHiddenNodes, getDefaultLayoutFileSuffix)
  • Added module exports for additional packages
  • Improved null-safety in AssociationOrLinkPartEdge.adjacentObjectNodeGreyed()
  • Simplified factory method implementations by removing unnecessary intermediate variables

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
NewObjectDiagram.java Main refactoring: encapsulation improvements, null-safety enhancements, code modernization, persistence refactoring, method implementations
AssociationOrLinkPartEdge.java Null-safety improvement and factory method simplification
module-info.java Added exports for elements, edges, event, and objectselection packages
Comments suppressed due to low confidence (1)

use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java:314

  • The onClosing() method has been removed but listeners registered at lines 288-289 (fParent.getModelBrowser().addHighlightChangeListener(this) and ModelBrowserSorting.getInstance().addSortChangeListener(this)) are never removed. This can cause memory leaks because the diagram instance cannot be garbage collected. Similar to ClassDiagram.java line 1448-1455, an onClosing() override should be added to remove these listeners and fParent's key listener (inputHandling added at line 290).
		fParent.getModelBrowser().addHighlightChangeListener(this);
		ModelBrowserSorting.getInstance().addSortChangeListener(this);
		fParent.addKeyListener(inputHandling);

		addMouseListener(inputHandling);
		addMouseListener(new MouseAdapter() {
			@Override
			public void mouseClicked(MouseEvent e) {
				if (e.getClickCount() == 2) {
					PlaceableNode pickedObjectNode = findNode(e.getX(), e.getY());
					if (pickedObjectNode instanceof ObjectNode on) {
						ObjectPropertiesView v = MainWindow.instance().showObjectPropertiesView();
						v.selectObject(on.object().name());
					}
				}
			}
		});

		addComponentListener(new ComponentAdapter() {
			public void componentResized(ComponentEvent e) {
				// need a new layouter to adapt to new window size
				fLayouter = null;
			}
		});

		startLayoutThread();
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1555 to +1556
@SuppressWarnings("unused")
private void restoreNodeEdges(PersistHelper helper, int version, Set<MObject> hiddenObjects) {
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @SuppressWarnings("unused") annotation is incorrect. This method is actually used by restorePlacementInfos at line 1500. The annotation should be removed as it's misleading and the method is not unused.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ahmedkha79
Copy link
Author

@copilot open a new pull request to apply changes based on the comments in this thread

ahmedkha79 and others added 5 commits February 18, 2026 20:08
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ahmedkha79
Copy link
Author

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 288 to 292
fParent.getModelBrowser().addHighlightChangeListener(this);
ModelBrowserSorting.getInstance().addSortChangeListener(this);
fParent.addKeyListener(inputHandling);

addMouseListener(inputHandling);
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagram registers itself as a highlight/sort listener and adds a KeyListener in the constructor, but the previous onClosing() cleanup hook is no longer present. This can leave listeners registered after the diagram is closed (memory leak / events delivered to disposed views) and also makes the javaFxCall flag ineffective. Reintroduce an onClosing() override that removes these listeners (similar to ClassDiagram/CommunicationDiagram).

Copilot uses AI. Check for mistakes.
}

// Helper: restore node-edge (link objects / node edges)
@SuppressWarnings("unused")
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @SuppressWarnings("unused") on restoreNodeEdges is misleading: the method is invoked from restorePlacementInfos. Please remove the suppression (or, if something else is unused here, suppress that more narrowly).

Suggested change
@SuppressWarnings("unused")

Copilot uses AI. Check for mistakes.
Comment on lines +1641 to +1658
List<MObject> connectedObjects = new LinkedList<>();
if (!helper.toFirstChild("connectedNode")) continue;
String objectName = helper.getElementStringValue();
MObject obj = fParent.system().state().objectByName(objectName);

if (obj != null)
connectedObjects.add(obj);

if (obj != null) connectedObjects.add(obj);
while (helper.toNextSibling("connectedNode")) {
objectName = helper.getElementStringValue();
obj = fParent.system().state().objectByName(objectName);

if (obj != null) {
connectedObjects.add(obj);
}
if (obj != null) connectedObjects.add(obj);
}

// Modified
if (assoc.associationEnds().size() != connectedObjects.size())
continue;

// n-ary links cannot be qualified therefore an empty list for the qualifer values is provided
MLink link = fParent.system().state().linkBetweenObjects(assoc, connectedObjects, Collections.<List<Value>>emptyList());

// Could be deleted
if (assoc.associationEnds().size() != connectedObjects.size()) continue;
MLink link = fParent.system().state().linkBetweenObjects(assoc, connectedObjects, Collections.emptyList());
if (link != null) {
DiamondNode node = visibleData.fNaryLinkToDiamondNodeMap.get(link);
helper.toParent();
node.restorePlacementInfo(helper, version);
}
DiamondNode node = visibleData.getNaryLinkToDiamondNodeMap().get(link);
if (node != null) {
helper.toParent();
node.restorePlacementInfo(helper, version);
}
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In restoreDiamondNodes(), after helper.toFirstChild("connectedNode") the navigator is only moved back to the parent (helper.toParent()) when node != null. If link == null or node == null, the navigator can remain positioned on a connectedNode child, which can break subsequent XPath iteration / value reads. Ensure the navigator is restored to the DiamondNode element for every loop iteration (e.g., call helper.toParent() in a finally block after processing connectedNode children).

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot implement suggestion

Comment on lines +1243 to +1253
popupMenu.insert(new JSeparator(), pos++);

final JMenu showHideCrop = new JMenu("Show/hide/crop objects");
final JMenu showProtocolStateMachine = new JMenu("Show protocol state machine...");
showProtocolStateMachine.setEnabled(false);
popupMenu.insert(showProtocolStateMachine, pos++);

if (!selectedObjects.isEmpty()) {
showHideCrop.add(fSelection.getSelectedObjectPathView("By path length...", selectedObjectsSet));
}
if (selectedObjects.size() == 1) {
final MObject obj = exactlyOne(selectedObjects);

showHideCrop.add(fSelection.getSelectionWithOCLViewAction());
showHideCrop.add(fSelection.getSelectionObjectView());
List<MProtocolStateMachine> sortedPSMs = new LinkedList<>(
obj.cls().getAllOwnedProtocolStateMachines());
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unionOfPopUpMenu() no longer adds the ObjectSelection submenus/actions (e.g., show/hide objects, show/hide links, links-by-kind, selection views) that previously provided important diagram navigation and bulk operations. This is a user-visible behavior change and isn’t mentioned in the PR description; please restore the removed menu entries or document/justify the intentional removal.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

for (MLink l : matchingLinks) {
if (Objects.equals(linkValue, l == null ? null : l.toString())) {
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant null check: The condition "l == null ? null : l.toString()" is unnecessary because l is already guaranteed to be non-null at this point in the code. The null check at line 1881 ensures all null links are skipped. Simplify to "Objects.equals(linkValue, l.toString())".

Suggested change
if (Objects.equals(linkValue, l == null ? null : l.toString())) {
if (Objects.equals(linkValue, l.toString())) {

Copilot uses AI. Check for mistakes.
MLinkEnd sourceEnd, MLinkEnd targetEnd, NewObjectDiagram diagram, MLink link) {
return BinaryAssociationOrLinkEdge.create(source, target, sourceEnd, targetEnd, diagram, link);
}
}
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onClosing() method has been removed but it contained critical cleanup logic for removing event listeners. This will cause memory leaks as the diagram instances will not be garbage collected because they remain registered as listeners. The method should be restored to clean up: fParent.getModelBrowser().removeHighlightChangeListener(this), fParent.removeKeyListener(inputHandling), and ModelBrowserSorting.getInstance().removeSortChangeListener(this). Additionally, the javaFxCall check and super.onClosing() call should be included.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ahmedkha79 and others added 3 commits February 18, 2026 21:15
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ahmedkha79
Copy link
Author

@copilot open a new pull request to apply changes based on the comments in this thread

@ahmedkha79
Copy link
Author

@copilot open a new pull request to apply changes based on the comments in this thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants