From ca87662d32e949b65c140bc8148271ec39901975 Mon Sep 17 00:00:00 2001 From: Jared Davis Date: Thu, 11 Jun 2026 08:43:08 -0400 Subject: [PATCH 1/5] Change getAll() to vars() Remove the Collections.unmodifiableMap() from vars() --- .../scijava/parsington/eval/AbstractEvaluator.java | 5 +++-- .../org/scijava/parsington/eval/Evaluator.java | 7 ++++--- .../parsington/eval/AbstractEvaluatorTest.java | 14 +++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java index 82502ba..172f632 100644 --- a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java +++ b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java @@ -89,9 +89,10 @@ public Object get(final String name) { return new Unresolved(name); } + @Override - public Map getAll() { - return Collections.unmodifiableMap(vars); + public Map vars() { + return vars; } @Override diff --git a/src/main/java/org/scijava/parsington/eval/Evaluator.java b/src/main/java/org/scijava/parsington/eval/Evaluator.java index 1be84d2..4409717 100644 --- a/src/main/java/org/scijava/parsington/eval/Evaluator.java +++ b/src/main/java/org/scijava/parsington/eval/Evaluator.java @@ -168,11 +168,12 @@ default Object get(final Variable v) { } /** - * Gets a map of all variable names and values. + * Gets the Evaluator variables. Not thread-safe. + * A map of all variable names and values. * - * @return A map from variable names to variable values. + * @return The map from variable names to variable values. */ - Map getAll(); + Map vars(); /** * Sets the value of a variable. diff --git a/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java b/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java index 62d5fc9..53b443a 100644 --- a/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java +++ b/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java @@ -84,16 +84,16 @@ public void testClear() { e.set("a", 1); e.set("b", 2); e.clear(); - assertEquals(new HashMap<>(), e.getAll()); + assertEquals(new HashMap<>(), e.vars()); assertThrows(IllegalArgumentException.class, () -> e.get("a")); assertThrows(IllegalArgumentException.class, () -> e.get("b")); } - /** Tests {@link Evaluator#getAll()} and {@link Evaluator#setAll(Map)}. */ + /** Tests {@link Evaluator#vars()} and {@link Evaluator#setAll(Map)}. */ @Test - public void testGetAllSetAll() { + public void testVarsSetAll() { final Evaluator e = createEvaluator(); - assertEquals(new HashMap<>(), e.getAll()); + assertEquals(new HashMap<>(), e.vars()); final Map vars = new HashMap<>(); vars.put("a", 1); @@ -101,7 +101,7 @@ public void testGetAllSetAll() { vars.put("c", 3.0); e.setAll(vars); - assertEquals(vars, e.getAll()); + assertEquals(vars, e.vars()); // Verify individual get still works after setAll. assertEquals(1, e.get("a")); @@ -110,9 +110,9 @@ public void testGetAllSetAll() { // Verify variables created at evaluation are accessible and correct. e.evaluate("d=a+c"); - assertTrue(e.getAll().containsKey("d")); + assertTrue(e.vars().containsKey("d")); final Object dVal = e.get("d"); assertEquals(4.0, dVal); - assertEquals(dVal, e.getAll().get("d")); + assertEquals(dVal, e.vars().get("d")); } } From e2b2155d0663c5e46e38ad341814441ebd640ba8 Mon Sep 17 00:00:00 2001 From: Jared Davis Date: Thu, 11 Jun 2026 08:43:49 -0400 Subject: [PATCH 2/5] Remove unused import --- src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java index 172f632..229e993 100644 --- a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java +++ b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java @@ -30,7 +30,6 @@ package org.scijava.parsington.eval; -import java.util.Collections; import java.util.HashMap; import java.util.Map; From e7dd5341375404c3751a8b23081c8cc551ae9049 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 17 Jul 2026 12:38:38 -0500 Subject: [PATCH 3/5] Improve javadoc for Evaluator#vars() function And place it before the more specific accessors and mutators. --- .../parsington/eval/AbstractEvaluator.java | 10 +++++----- .../scijava/parsington/eval/Evaluator.java | 19 +++++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java index 229e993..cf2544d 100644 --- a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java +++ b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java @@ -72,6 +72,11 @@ public void setStrict(final boolean strict) { this.strict = strict; } + @Override + public Map vars() { + return vars; + } + @Override public Object get(final String name) { // NB: Here, we look up the key twice: once with containsKey, then @@ -89,11 +94,6 @@ public Object get(final String name) { } - @Override - public Map vars() { - return vars; - } - @Override public void set(final String name, final Object value) { vars.put(name, value); diff --git a/src/main/java/org/scijava/parsington/eval/Evaluator.java b/src/main/java/org/scijava/parsington/eval/Evaluator.java index 4409717..4c63977 100644 --- a/src/main/java/org/scijava/parsington/eval/Evaluator.java +++ b/src/main/java/org/scijava/parsington/eval/Evaluator.java @@ -112,6 +112,17 @@ public interface Evaluator { */ Object evaluate(SyntaxTree syntaxTree); + /** + * Gets all variables as a mutable {@link Map}. Not thread-safe. + *

+ * This map is a view, not a copy—i.e., changes to this map alter the + * evaluator's actual variable values. + *

+ * + * @return The map from variable names to variable values. + */ + Map vars(); + /** * Gets the value of a token. For variables, returns the value of the * variable, throwing an exception if the variable is not set. For literals, @@ -167,14 +178,6 @@ default Object get(final Variable v) { return get(v.getToken()); } - /** - * Gets the Evaluator variables. Not thread-safe. - * A map of all variable names and values. - * - * @return The map from variable names to variable values. - */ - Map vars(); - /** * Sets the value of a variable. * From 5e1e94559111f38d8bfcbcee6c309d9027984973 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 17 Jul 2026 12:49:58 -0500 Subject: [PATCH 4/5] Push map-like Evaluator methods impls up to iface --- .../parsington/eval/AbstractEvaluator.java | 21 ------------------- .../scijava/parsington/eval/Evaluator.java | 17 ++++++++++----- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java index cf2544d..4be9e8e 100644 --- a/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java +++ b/src/main/java/org/scijava/parsington/eval/AbstractEvaluator.java @@ -93,25 +93,4 @@ public Object get(final String name) { return new Unresolved(name); } - - @Override - public void set(final String name, final Object value) { - vars.put(name, value); - } - - @Override - public void setAll(final Map map) { - vars.putAll(map); - } - - @Override - public Object remove(final String name) { - return vars.remove(name); - } - - @Override - public void clear() { - vars.clear(); - } - } diff --git a/src/main/java/org/scijava/parsington/eval/Evaluator.java b/src/main/java/org/scijava/parsington/eval/Evaluator.java index 4c63977..9395dd5 100644 --- a/src/main/java/org/scijava/parsington/eval/Evaluator.java +++ b/src/main/java/org/scijava/parsington/eval/Evaluator.java @@ -164,7 +164,9 @@ default Variable var(final Object token) { * @param name The name of the variable whose value you want to set. * @param value The value to assign to the variable. */ - void set(String name, Object value); + default void set(String name, Object value) { + vars().put(name, value); + } /** * Gets the value of a variable. @@ -193,7 +195,9 @@ default void set(final Variable v, final Object value) { * * @param map A map from variable names to variable values. */ - void setAll(Map map); + default void setAll(Map map) { + vars().putAll(map); + } /** * Removes the named variable. @@ -202,12 +206,15 @@ default void set(final Variable v, final Object value) { * @return The previous variables value associated with name, * or null if the name did not exist. */ - Object remove(String name); + default Object remove(String name) { + return vars().remove(name); + } /** * Clears all the variables. - * */ - void clear(); + default void clear() { + vars().clear(); + } } From b5e22bd7f4754ce7fad9d77207e967e823ab1acd Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 17 Jul 2026 13:09:05 -0500 Subject: [PATCH 5/5] Improve AbstractEvaluator#testClear Check size before and after, rather than allocating a new object. --- .../org/scijava/parsington/eval/AbstractEvaluatorTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java b/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java index 53b443a..5b66364 100644 --- a/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java +++ b/src/test/java/org/scijava/parsington/eval/AbstractEvaluatorTest.java @@ -83,8 +83,9 @@ public void testClear() { final Evaluator e = createEvaluator(); e.set("a", 1); e.set("b", 2); + assertEquals(2, e.vars().size()); e.clear(); - assertEquals(new HashMap<>(), e.vars()); + assertEquals(0, e.vars().size()); assertThrows(IllegalArgumentException.class, () -> e.get("a")); assertThrows(IllegalArgumentException.class, () -> e.get("b")); }