Skip to content

rm com.google.guava from main bundles#1189

Open
stbischof wants to merge 2 commits into
eclipse-elk:masterfrom
stbischof:master
Open

rm com.google.guava from main bundles#1189
stbischof wants to merge 2 commits into
eclipse-elk:masterfrom
stbischof:master

Conversation

@stbischof
Copy link
Copy Markdown

@stbischof stbischof commented Apr 25, 2026 •
edited
Loading

  • Lists.newArrayList(), Maps.newHashMap(), Sets.newHashSet() → JDK constructors
  • Strings.isNullOrEmpty(s) → (s == null || s.isEmpty())
  • Joiner.on(s).join(c) → c.stream()...Collectors.joining(...)
  • Guava Predicate/Function/Optional → java.util.function.* / java.util.Optional
  • DoubleMath.fuzzyEquals/fuzzyCompare → public static helpers added to CompareFuzzy
  • ImmutableMap.of, ImmutableSet.of → Map.of / Set.of
  • Multimap/HashMultimap/ArrayListMultimap/LinkedHashMultimap/ListMultimap → Map<K, List> / Map<K, Set> with computeIfAbsent
  • Table/ArrayTable → nested Map<R, Map<C, V>> (using EnumMap for enum keys)
  • Range.closed(a, b) → plain int constants
  • Iterables.concat/filter/size/stream and Lists.reverse → centralized helpers in LGraphUtil

#1190

@stbischof
rm com.google.guava from main bundles
0eff643 
 - Lists.newArrayList(), Maps.newHashMap(), Sets.newHashSet() → JDK constructors
  - Strings.isNullOrEmpty(s) → (s == null || s.isEmpty())
  - Joiner.on(s).join(c) → c.stream()...Collectors.joining(...)
  - Guava Predicate/Function/Optional → java.util.function.* / java.util.Optional
  - DoubleMath.fuzzyEquals/fuzzyCompare → public static helpers added to CompareFuzzy
  - ImmutableMap.of, ImmutableSet.of → Map.of / Set.of
  - Multimap/HashMultimap/ArrayListMultimap/LinkedHashMultimap/ListMultimap → Map<K, List<V>> / Map<K, Set<V>> with computeIfAbsent
  - Table/ArrayTable → nested Map<R, Map<C, V>> (using EnumMap for enum keys)
  - Range.closed(a, b) → plain int constants
  - Iterables.concat/filter/size/stream and Lists.reverse → centralized helpers in LGraphUtil

Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
soerendomroes
soerendomroes reviewed Apr 27, 2026
View reviewed changes
Comment thread plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/graph/LGraphUtil.java
Copy link
Copy Markdown
Contributor

@soerendomroes soerendomroes Apr 27, 2026

Choose a reason for hiding this comment

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

Is there a source for the added function, i.e. are they copied from com.google.common?

Comment thread ...lk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/loops/ordering/PortRestorer.java
Comment on lines 75 to +95
@@ -86,14 +86,13 @@ public void restorePorts(final SelfLoopHolder slHolder, final IElkProgressMonito
* of port side and port side area in {@link #targetAreas}.
*/
private void initTargetAreas() {
// An array table is more efficient than other tables, but requires all row and column keys to be specified in
// advance
targetAreas = ArrayTable.create(Arrays.asList(PortSide.values()), Arrays.asList(PortSideArea.values()));

targetAreas = new EnumMap<>(PortSide.class);
for (PortSide side : PortSide.values()) {
Map<PortSideArea, List<SelfLoopPort>> areaMap = new EnumMap<>(PortSideArea.class);
for (PortSideArea area : PortSideArea.values()) {
targetAreas.put(side, area, new ArrayList<>());
areaMap.put(area, new ArrayList<>());
}
targetAreas.put(side, areaMap);
Copy link
Copy Markdown
Contributor

@soerendomroes soerendomroes Apr 27, 2026

Choose a reason for hiding this comment

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

This seems to be a semantic change?

Comment thread .../org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/options/InternalProperties.java
Comment on lines +335 to +336
public static final IProperty<Map<LEdge, Set<CrossHierarchyEdge>>> CROSS_HIERARCHY_MAP =
new Property<Map<LEdge, Set<CrossHierarchyEdge>>>("crossHierarchyMap");
Copy link
Copy Markdown
Contributor

@soerendomroes soerendomroes Apr 27, 2026

Choose a reason for hiding this comment

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

Map and Set might mess with the iteration order.

Maybe this was also problematic using MultiMap?

@soerendomroes
Copy link
Copy Markdown
Contributor

soerendomroes commented Apr 27, 2026

Due to current Xtext's limitations (2.36.0), source and target compatibility have been set to 11.

We might need an Xtext upgrade.

Tests run: 11, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.002 s <<< FAILURE! -- in org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.givenMoreHierarchicalNodes2_DoesNotSwitchPorts -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.distributePortsWhileSweeping_givenTwoHierarchicalNodesInOneLayer -- Time elapsed: 0.001 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.givenDoubleCrossBetweenCompoundAndNonCompoundNodes_SwitchesPorts -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.distributePorts_GivenNoPortsOnRightSide_NothingHappens -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.distributePorts_fixedPortOrder_NoChange -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.givenMoreHierarchicalNodes_DoesNotSwitchPorts -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.noChange -- Time elapsed: 0 s
org.eclipse.elk.alg.layered.p3order.GreedyPortDistributorTest.distributePorts_GivenMultipleCrossingsOnWesternSide_RemoveCrossing -- Time elapsed: 0.001 s <<< FAILURE!
java.lang.AssertionError:

Expected: is <[p_1[n_0] << 0[n_2], p_2[n_0] << 0[n_1], p_0[n_0] << 0[n_0]]>
but: was <[p_0[n_0] << 0[n_0], p_1[n_0] << 0[n_2], p_2[n_0] << 0[n_1]]>
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)

Something changes the port ordering. I am not sure, which implementation is better, as the p_0, p_1 and p_2 ordering looks better in terms of model order.

The errors are harder to track down but something seems to be missing here.

Cannot invoke "java.lang.Integer.intValue()" because the return value of "java.util.Map.get(Object)" is null

Maybe some default value thing?

@stbischof
Copy link
Copy Markdown
Author

stbischof commented Apr 29, 2026

will look at this PR next week again

@stbischof
fixup the method
8fb6a9b 
Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soerendomroes soerendomroes soerendomroes left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stbischof @soerendomroes