Conversation
- 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
…ion fields as final (small cleanup)
…ct map to avoid private-field access
…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.
… into refactor/NewObjectDiagram-fixes
There was a problem hiding this comment.
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.
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
| @SuppressWarnings("unused") | ||
| private void restoreNodeEdges(PersistHelper helper, int version, Set<MObject> hiddenObjects) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
…gram/NewObjectDiagram.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…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>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
There was a problem hiding this comment.
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.
| fParent.getModelBrowser().addHighlightChangeListener(this); | ||
| ModelBrowserSorting.getInstance().addSortChangeListener(this); | ||
| fParent.addKeyListener(inputHandling); | ||
|
|
||
| addMouseListener(inputHandling); |
There was a problem hiding this comment.
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).
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // Helper: restore node-edge (link objects / node edges) | ||
| @SuppressWarnings("unused") |
There was a problem hiding this comment.
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).
| @SuppressWarnings("unused") |
| 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); | ||
| } |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| 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()); |
There was a problem hiding this comment.
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.
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Show resolved
Hide resolved
…Objects.equals; suppress unused warnings
There was a problem hiding this comment.
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.
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| for (MLink l : matchingLinks) { | ||
| if (Objects.equals(linkValue, l == null ? null : l.toString())) { |
There was a problem hiding this comment.
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())".
| if (Objects.equals(linkValue, l == null ? null : l.toString())) { | |
| if (Objects.equals(linkValue, l.toString())) { |
| MLinkEnd sourceEnd, MLinkEnd targetEnd, NewObjectDiagram diagram, MLink link) { | ||
| return BinaryAssociationOrLinkEdge.create(source, target, sourceEnd, targetEnd, diagram, link); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
use-gui/src/main/java/org/tzi/use/gui/views/diagrams/objectdiagram/NewObjectDiagram.java
Show resolved
Hide resolved
…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>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gram/NewObjectDiagram.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
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)
ShowObjectPropertiesViewMouseListener.instanceofwo sinnvoll).finalmarkiert.ObjectDiagramData-Accessoren (getVisibleData/getHiddenData).getLinkByValue,getHiddenNodes,getDefaultLayoutFileSuffix).createAction-Helper hinzugefügt, um wiederkehrende Action/Listener-Boilerplate zu reduzieren.instanceof,computeIfAbsent/computeIfPresent.@SuppressWarnings("unused"), wo API-Overloads für Kompatibilität bestehen.isInstance()Checks und Entfernung von doppelten containsKey/get-Kombinationen.Betroffene Hauptdateien
Risiken & Tests
Anmerkung