refactor: Pre-compute PrecedenceLevelInfo (Big Commit)

MarcellPerger1 · MarcellPerger1 · commit 208609ef4f42 · 2024-04-21T21:43:38.000+01:00
diff --git a/src/main/java/net/marcellperger/mathexpr/BinaryOperation.java b/src/main/java/net/marcellperger/mathexpr/BinaryOperation.java
@@ -8,4 +8,8 @@ public interface BinaryOperation extends MathSymbol {
     default String fmt() {
         return "%s(%s, %s)".formatted(this.getClass().getSimpleName(), getLeft().fmt(), getRight().fmt());
     }
+
+    static MathSymbol construct(MathSymbol left, SymbolInfo op, MathSymbol right) {
+        return op.getBiConstructor().construct(left, right);
+    }
 }
diff --git a/src/main/java/net/marcellperger/mathexpr/PrecedenceLevelInfo.java b/src/main/java/net/marcellperger/mathexpr/PrecedenceLevelInfo.java
@@ -0,0 +1,59 @@
+package net.marcellperger.mathexpr;
+
+import net.marcellperger.mathexpr.util.Util;
+import net.marcellperger.mathexpr.util.UtilCollectors;
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Data for a specific precedence level
+ */
+public class PrecedenceLevelInfo {
+    public Set<SymbolInfo> symbols;
+    public Map<String, SymbolInfo> infixToSymbolMap;
+    public List<String> sortedInfixes;
+    public @Nullable GroupingDirection dirn;
+    public int precedence;
+
+    /**
+     * @param precedence_ The precedence level (gets data from SymbolInfo)
+     */
+    public PrecedenceLevelInfo(int precedence_) {
+        precedence = precedence_;
+        symbols = SymbolInfo.PREC_TO_INFO_MAP.get(precedence);
+        // Shouldn't be passed an empty level in the first place
+        Util.requireNonEmptyNonNull(symbols, "Cannot create PrecedenceLevelInfo for a level with no items");
+        dirn = symbols.stream()
+            .map(sm -> sm.groupingDirection).collect(UtilCollectors.singleDistinctItem());
+        try {
+            infixToSymbolMap = symbols.stream().collect(
+                Collectors.toMap(
+                    info -> Util.requireNonNull(info.infix, new NullInfixException()),
+                    info -> info));
+        } catch (NullInfixException e) {
+            // a precedence level should really be uniform so if any infix is null,
+            //  we set this to null. This may help avoid any subtle bugs later.
+            infixToSymbolMap = null;
+            sortedInfixes = null;
+        }
+        if(infixToSymbolMap != null) {
+            sortedInfixes = infixToSymbolMap.keySet().stream()
+                .sorted(Comparator.comparingInt(String::length).reversed()).toList();
+        }
+    }
+
+    @Contract("_ -> new")
+    public static Map.@NotNull Entry<Integer, PrecedenceLevelInfo> newMapEntry(int precedence) {
+        return Util.makeEntry(precedence, new PrecedenceLevelInfo(precedence));
+    }
+
+    /** Marker exception that we throw to signal that there should be no infix info at all */
+    private static class NullInfixException extends RuntimeException {}
+}
diff --git a/src/main/java/net/marcellperger/mathexpr/SymbolInfo.java b/src/main/java/net/marcellperger/mathexpr/SymbolInfo.java
@@ -1,6 +1,7 @@
 package net.marcellperger.mathexpr;
 
 import net.marcellperger.mathexpr.util.Util;
+import net.marcellperger.mathexpr.util.UtilCollectors;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -30,6 +31,7 @@ public enum SymbolInfo {
     public static final Map<Integer, Set<SymbolInfo>> PREC_TO_INFO_MAP;
     public static final List<Entry<Integer, Set<SymbolInfo>>> PREC_SORTED_INFO;
     public static final int MAX_PRECEDENCE;
+    public static final Map<Integer, PrecedenceLevelInfo> PREC_LEVELS_INFO;
 
     public final int precedence;  // TODO make this Integer
     public final Class<? extends MathSymbol> cls;
@@ -106,5 +108,6 @@ public enum SymbolInfo {
         PREC_TO_INFO_MAP = Arrays.stream(values()).collect(Collectors.groupingBy(s -> s.precedence, Collectors.toUnmodifiableSet()));
         PREC_SORTED_INFO = PREC_TO_INFO_MAP.entrySet().stream().sorted(Comparator.comparingInt(Entry::getKey)).toList();
         MAX_PRECEDENCE = PREC_SORTED_INFO.getLast().getKey();
+        PREC_LEVELS_INFO = PREC_TO_INFO_MAP.keySet().stream().map(PrecedenceLevelInfo::newMapEntry).collect(UtilCollectors.entriesToMap());
     }
 }
diff --git a/src/main/java/net/marcellperger/mathexpr/parser/Parser.java b/src/main/java/net/marcellperger/mathexpr/parser/Parser.java
@@ -3,17 +3,17 @@
 import net.marcellperger.mathexpr.*;
 import net.marcellperger.mathexpr.util.Pair;
 import net.marcellperger.mathexpr.util.Util;
-import net.marcellperger.mathexpr.util.UtilCollectors;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Range;
 
 import java.nio.CharBuffer;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
 import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 
 
 public class Parser {
@@ -79,52 +79,39 @@ public MathSymbol parseExpr() throws ExprParseException {
 
     public MathSymbol parseInfixPrecedenceLevel(int level) throws ExprParseException {
         discardWhitespace();
-        if(level == 0) {
-            return parseParensOrLiteral();
-        }
-        Set<SymbolInfo> symbols = SymbolInfo.PREC_TO_INFO_MAP.get(level);
-        if(symbols == null || symbols.isEmpty()) return parseInfixPrecedenceLevel(level - 1);
-        @Nullable GroupingDirection dirn = symbols.stream()
-            .map(sm -> sm.groupingDirection).collect(UtilCollectors.singleDistinctItem());
-        Map<String, SymbolInfo> infixToSymbolInfo = symbols.stream().collect(  // TODO pre-compute/cache these
-            Collectors.toUnmodifiableMap(
-                si -> Objects.requireNonNull(si.infix, "null infix not allowed for parseInfixPrecedenceLevel"),
-                Function.identity()));
-        String[] infixesToFind = sortedByLength(infixToSymbolInfo.keySet().toArray(String[]::new));
+        if(level == 0) return parseParensOrLiteral();
+        PrecedenceLevelInfo precInfo = SymbolInfo.PREC_LEVELS_INFO.get(level);
+        if (precInfo == null) return parseInfixPrecedenceLevel(level - 1);
         MathSymbol left = parseInfixPrecedenceLevel(level - 1);
-        if(dirn == GroupingDirection.RightToLeft) {
-            return parseInfixPrecedenceLevel_RTL(left, level, infixesToFind, infixToSymbolInfo);
+        if(precInfo.dirn == GroupingDirection.RightToLeft) {
+            return parseInfixPrecedenceLevel_RTL(left, precInfo);
         }
-        return parseInfixPrecedenceLevel_LTR(left, level, infixesToFind, infixToSymbolInfo);
+        return parseInfixPrecedenceLevel_LTR(left, precInfo);
         // TODO what if dirn == null? Maybe just disallow the ambiguous case of > 2 operands in same level
     }
 
-    private MathSymbol parseInfixPrecedenceLevel_RTL(MathSymbol left, int level, String[] infixesToFind,
-                                                     Map<String, SymbolInfo> infixToSymbolInfo) throws ExprParseException {
-        // TODO: refactor this mess - 2 separate loops?
-        //  I feel like it should be doable w/ one loop but that may involve risking NullPointerException
-        //  by setting some members of LeftRightBinaryOperation to null
-        //  Actually, this first loop could be common between them if we make the other path 2 loops as well
+    private MathSymbol parseInfixPrecedenceLevel_RTL(MathSymbol left, PrecedenceLevelInfo precInfo) throws ExprParseException {
         String op;
         List<Pair<SymbolInfo, MathSymbol>> otherOps = new ArrayList<>();
         discardWhitespace();
-        while((op = discardMatchesNextAny_optionsSorted(infixesToFind)) != null) {
-            otherOps.add(new Pair<>(Util.getNotNull(infixToSymbolInfo, op), parseInfixPrecedenceLevel(level - 1)));
+        while((op = discardMatchesNextAny_optionsSorted(precInfo.sortedInfixes)) != null) {
+            otherOps.add(new Pair<>(
+                Util.getNotNull(precInfo.infixToSymbolMap, op),
+                parseInfixPrecedenceLevel(precInfo.precedence - 1)));
             discardWhitespace();
         }
         return otherOps.reversed().stream().reduce((rightpair, leftpair) ->
             leftpair.asVars((preOp, argL) ->
-                new Pair<>(preOp, rightpair.asVars((midOp, argR) -> midOp.getBiConstructor().construct(argL, argR))))
-        ).map(p -> p.asVars((midOp, argR) -> midOp.getBiConstructor().construct(left, argR))).orElse(left);
+                new Pair<>(preOp, rightpair.asVars((midOp, argR) -> BinaryOperation.construct(argL, midOp, argR))))
+        ).map(p -> p.asVars((midOp, argR) -> BinaryOperation.construct(left, midOp, argR))).orElse(left);
     }
 
-    private MathSymbol parseInfixPrecedenceLevel_LTR(MathSymbol left, int level, String[] infixesToFind,
-                                                     Map<String, SymbolInfo> infixToSymbolInfo) throws ExprParseException {
+    private MathSymbol parseInfixPrecedenceLevel_LTR(MathSymbol left, PrecedenceLevelInfo precInfo) throws ExprParseException {
         String op;
         discardWhitespace();
-        while((op = discardMatchesNextAny_optionsSorted(infixesToFind)) != null) {
-            left = Util.getNotNull(infixToSymbolInfo, op).getBiConstructor()
-                .construct(left, parseInfixPrecedenceLevel(level - 1));
+        while((op = discardMatchesNextAny_optionsSorted(precInfo.sortedInfixes)) != null) {
+            left = BinaryOperation.construct(
+                left, Util.getNotNull(precInfo.infixToSymbolMap, op), parseInfixPrecedenceLevel(precInfo.precedence - 1));
             discardWhitespace();
         }
         return left;
@@ -182,24 +169,24 @@ protected boolean matchesNext(@NotNull String expected) {
         return src.startsWith(expected, /*start*/idx);
     }
     
-    private @NotNull String @NotNull [] sortedByLength(@NotNull String @NotNull[] arr) {
-        return Arrays.stream(arr).sorted(Comparator.comparingInt(String::length).reversed()).toArray(String[]::new);
+    private @NotNull List<@NotNull String> sortedByLength(@NotNull List<@NotNull String> arr) {
+        return arr.stream().sorted(Comparator.comparingInt(String::length).reversed()).toList();
     }
-    private @Nullable String matchesNextAny_optionsSorted(@NotNull String... expected) {
-        return Arrays.stream(expected).filter(this::matchesNext).findFirst().orElse(null);
+    private @Nullable String matchesNextAny_optionsSorted(@NotNull List<@NotNull String> expected) {
+        return expected.stream().filter(this::matchesNext).findFirst().orElse(null);
     }
-    private @Nullable String discardMatchesNextAny_optionsSorted(@NotNull String... expected) {
+    private @Nullable String discardMatchesNextAny_optionsSorted(@NotNull List<@NotNull String> expected) {
         String s = matchesNextAny_optionsSorted(expected);
         if(s != null) discardN(s.length());
         return s;
     }
     @SuppressWarnings("unused")
-    protected @Nullable String matchesNextAny(@NotNull String... expected) {
+    protected @Nullable String matchesNextAny(@NotNull List<@NotNull String> expected) {
         // Try to longer ones first, then shorter ones.
         return matchesNextAny_optionsSorted(sortedByLength(expected));
     }
     @SuppressWarnings("unused")
-    protected @Nullable String discardMatchesNextAny(@NotNull String... expected) {
+    protected @Nullable String discardMatchesNextAny(@NotNull List<@NotNull String> expected) {
         // Try to longer ones first, then shorter ones.
         return discardMatchesNextAny_optionsSorted(sortedByLength(expected));
     }
diff --git a/src/main/java/net/marcellperger/mathexpr/util/Util.java b/src/main/java/net/marcellperger/mathexpr/util/Util.java
@@ -49,6 +49,13 @@ protected Util() {}
         return chainNulls(chainNulls(value, f0, f1, f2, f3), f4);
     }
 
+    /** This exists solely to be able to provide a custom error type */
+    @Contract(value = "null, _ -> fail; !null, _ -> param1", pure = true)
+    public static <T> @NotNull T requireNonNull(T obj, RuntimeException exc) {
+        if(obj == null) throw exc;
+        return obj;
+    }
+
     @SuppressWarnings("UnusedReturnValue")
     @Contract("_ -> param1")
     public static <C extends Collection<?>> @NotNull C requireNonEmpty(@NotNull C collection) {
@@ -59,23 +66,29 @@ protected Util() {}
     @SuppressWarnings("UnusedReturnValue")
     @Contract("null -> fail; _ -> param1")
     public static <C extends Collection<?>> @NotNull C requireNonEmptyNonNull(C collection) {
-        Objects.requireNonNull(collection);
-        // IllegalArgumentException might not always be appropriate
-        if(collection.isEmpty()) throw new IllegalArgumentException("Argument must not be empty");
-        return collection;
+        return requireNonEmpty(Objects.requireNonNull(collection));
     }
     @Contract("_,_ -> param1")
     public static <C extends Collection<?>> @NotNull C requireNonEmpty(@NotNull C collection, String msg) {
         // IllegalArgumentException might not always be appropriate
         if(collection.isEmpty()) throw new IllegalArgumentException(msg);
         return collection;
     }
+    @SuppressWarnings("UnusedReturnValue")
+    @Contract("null, _ -> fail; _, _ -> param1")
+    public static <C extends Collection<?>> @NotNull C requireNonEmptyNonNull(C collection, String msg) {
+        return requireNonEmpty(Objects.requireNonNull(collection), msg);
+    }
     @Contract("_,_ -> param1")
     public static <C extends Collection<?>> @NotNull C requireNonEmpty(@NotNull C collection, RuntimeException exc) {
         // IllegalArgumentException might not always be appropriate
         if(collection.isEmpty()) throw exc;
         return collection;
     }
+    @Contract("null, _ -> fail; _, _ -> param1")
+    public static <C extends Collection<?>> @NotNull C requireNonEmptyNonNull(C collection, RuntimeException exc) {
+        return requireNonEmpty(Objects.requireNonNull(collection), exc);
+    }
 
     @Contract("_, _ -> param1")
     public static<T> T expectOrFail(T value, @NotNull Predicate<T> predicate) {
@@ -167,6 +180,11 @@ public static void realAssert(boolean b, String msg, Throwable cause) {
         return new AbstractMap.SimpleImmutableEntry<>(k, v);
     }
 
+    @Contract(pure = true)
+    public static <K, V, R> @NotNull Function<Map.Entry<K, V>, Map.Entry<K, R>> valueMapper(Function<V, R> mapper) {
+        return en -> makeEntry(en.getKey(), mapper.apply(en.getValue()));
+    }
+
     public static<T> T getOnlyItem(@Flow(sourceIsContainer = true) @NotNull Collection<T> c) {
         if(c.size() != 1) throw new CollectionSizeException("Expected collection to have 1 item");
         return c.iterator().next();
diff --git a/src/main/java/net/marcellperger/mathexpr/util/UtilCollectors.java b/src/main/java/net/marcellperger/mathexpr/util/UtilCollectors.java
@@ -3,6 +3,7 @@
 import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.stream.Collector;
@@ -17,7 +18,9 @@ protected UtilCollectors() {}  // prevent instantiation, allow subclassing
         return Collectors.collectingAndThen(Collectors.toList(), Util::getOnlyItem);
     }
     public static <T> @NotNull Collector<T, ?, T> singleDistinctItem() {
-        return Collectors.collectingAndThen(Collectors.toSet(), Util::getOnlyItem);
+        // We can't use toSet() as that doesn't guarantee null items are allowed
+        // although https://stackoverflow.com/q/47212902/19115554
+        return Collectors.collectingAndThen(Collectors.toCollection(HashSet::new), Util::getOnlyItem);
     }
 
     public static <K, V> @NotNull Collector<Entry<K, V>, ?, Map<K, V>> entriesToUnmodifiableMap() {

Original file line number	Diff line number	Diff line change
`@@ -8,4 +8,8 @@ public interface BinaryOperation extends MathSymbol {`
`8`	`8`	`default String fmt() {`
`9`	`9`	`return "%s(%s, %s)".formatted(this.getClass().getSimpleName(), getLeft().fmt(), getRight().fmt());`
`10`	`10`	`}`
	`11`	`+`
	`12`	`+ static MathSymbol construct(MathSymbol left, SymbolInfo op, MathSymbol right) {`
	`13`	`+ return op.getBiConstructor().construct(left, right);`
	`14`	`+ }`
`11`	`15`	`}`