Refactor to use Statistics.median method

Signed-off-by: Jimmy Tanagra <[email protected]>
openhab · Aug 24, 2024 · cfd87ca · cfd87ca
1 parent 624850b
commit cfd87ca
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 27 deletions.
diff --git a/...s/org.openhab.core/src/main/java/org/openhab/core/internal/items/GroupFunctionHelper.java b/...s/org.openhab.core/src/main/java/org/openhab/core/internal/items/GroupFunctionHelper.java
@@ -92,7 +92,7 @@ private GroupFunction createDimensionGroupFunction(GroupFunctionDTO function, @N
             case "AVG":
                 return new QuantityTypeArithmeticGroupFunction.Avg(dimension);
             case "MEDIAN":
-                return new QuantityTypeArithmeticGroupFunction.Median(dimension);
+                return new QuantityTypeArithmeticGroupFunction.Median(dimension, baseItem);
             case "SUM":
                 return new QuantityTypeArithmeticGroupFunction.Sum(dimension);
             case "MIN":

diff --git a/...rg.openhab.core/src/main/java/org/openhab/core/library/types/ArithmeticGroupFunction.java b/...rg.openhab.core/src/main/java/org/openhab/core/library/types/ArithmeticGroupFunction.java
@@ -26,6 +26,7 @@
 import org.openhab.core.items.Item;
 import org.openhab.core.types.State;
 import org.openhab.core.types.UnDefType;
+import org.openhab.core.util.Statistics;
 
 /**
  * This interface is only a container for functions that require the core type library
@@ -266,15 +267,11 @@ public Median() {
         @Override
         public State calculate(@Nullable Set<Item> items) {
             if (items != null) {
-                List<DecimalType> states = items.stream().map(item -> item.getStateAs(DecimalType.class))
-                        .filter(Objects::nonNull).sorted((s1, s2) -> s1.compareTo(s2)).toList();
-                int size = states.size();
-                if (size % 2 == 1) {
-                    return states.get(size / 2);
-                } else if (size > 0) {
-                    return new DecimalType(
-                            states.get(size / 2 - 1).toBigDecimal().add(states.get(size / 2).toBigDecimal())
-                                    .divide(BigDecimal.valueOf(2), MathContext.DECIMAL128));
+                List<BigDecimal> states = items.stream().map(item -> item.getStateAs(DecimalType.class))
+                        .filter(Objects::nonNull).map(DecimalType::toBigDecimal).toList();
+                BigDecimal median = Statistics.median(states);
+                if (median != null) {
+                    return new DecimalType(median);
                 }
             }
             return UnDefType.UNDEF;

diff --git a/...ore/src/main/java/org/openhab/core/library/types/QuantityTypeArithmeticGroupFunction.java b/...ore/src/main/java/org/openhab/core/library/types/QuantityTypeArithmeticGroupFunction.java
@@ -14,11 +14,12 @@
 
 import java.math.BigDecimal;
 import java.math.MathContext;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Objects;
 import java.util.Set;
 
 import javax.measure.Quantity;
+import javax.measure.Unit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -28,6 +29,7 @@
 import org.openhab.core.library.items.NumberItem;
 import org.openhab.core.types.State;
 import org.openhab.core.types.UnDefType;
+import org.openhab.core.util.Statistics;
 
 /**
  * This interface is a container for dimension based functions that require {@link QuantityType}s for its calculations.
@@ -118,26 +120,42 @@ public State calculate(@Nullable Set<Item> items) {
      */
     class Median extends DimensionalGroupFunction {
 
-        public Median(Class<? extends Quantity<?>> dimension) {
+        private @Nullable Item baseItem;
+
+        public Median(Class<? extends Quantity<?>> dimension, @Nullable Item baseItem) {
             super(dimension);
+            this.baseItem = baseItem;
         }
 
         @Override
         @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
             if (items != null) {
-                List<QuantityType> states = items.stream().filter(item -> isSameDimension(item))
-                        .map(item -> item.getStateAs(QuantityType.class)).filter(Objects::nonNull)
-                        .sorted((s1, s2) -> s1.compareTo(s2)).toList();
-                int size = states.size();
-                if (size % 2 == 1) {
-                    return states.get(size / 2);
-                } else if (size > 0) {
-                    QuantityType state1 = states.get(size / 2 - 1);
-                    QuantityType state2 = states.get(size / 2).toInvertibleUnit(state1.getUnit());
-                    BigDecimal result = state1.add(state2).toBigDecimal().divide(BigDecimal.valueOf(2),
-                            MathContext.DECIMAL128);
-                    return new QuantityType(result, state1.getUnit());
+                List<BigDecimal> values = new ArrayList<>();
+                Unit<?> unit = null;
+                if (baseItem instanceof NumberItem numberItem) {
+                    unit = numberItem.getUnit();
+                }
+                for (Item item : items) {
+                    if (!isSameDimension(item)) {
+                        continue;
+                    }
+                    QuantityType itemState = item.getStateAs(QuantityType.class);
+                    if (itemState == null) {
+                        continue;
+                    }
+                    if (unit == null) {
+                        unit = itemState.getUnit(); // set it to the first item's unit
+                    }
+                    values.add(itemState.toInvertibleUnit(unit).toBigDecimal());
+                }
+
+                if (!values.isEmpty()) {
+                    BigDecimal median = Statistics.median(values);
+                    if (median != null) {
+                        return new QuantityType<>(median, unit);
+                    }
+
                 }
             }
             return UnDefType.UNDEF;

diff --git a/...src/test/java/org/openhab/core/library/types/QuantityTypeArithmeticGroupFunctionTest.java b/...src/test/java/org/openhab/core/library/types/QuantityTypeArithmeticGroupFunctionTest.java
@@ -191,6 +191,11 @@ static Stream<Arguments> medianTestSource() {
                         List.of(new QuantityType("100 °C"), UnDefType.NULL, new QuantityType("200 °C"), UnDefType.UNDEF,
                                 new QuantityType("300 °C"), new QuantityType("400 °C")), //
                         new QuantityType("250 °C")), //
+                // mixed units. 200 °C = 392 °F
+                arguments( //
+                        List.of(new QuantityType("100 °C"), UnDefType.NULL, new QuantityType("392 °F"), UnDefType.UNDEF,
+                                new QuantityType("300 °C"), new QuantityType("400 °C")), //
+                        new QuantityType("250 °C")), //
                 arguments( //
                         List.of(new QuantityType("100 °C"), UnDefType.NULL, new QuantityType("200 °C"), UnDefType.UNDEF,
                                 new QuantityType("300 °C")), //
@@ -215,13 +220,13 @@ public void testMedianFunctionQuantityType(List<State> states, State expected) {
                 .map(state -> createNumberItem("TestItem" + index.getAndIncrement(), Temperature.class, state))
                 .collect(Collectors.toSet());
 
-        GroupFunction function = new QuantityTypeArithmeticGroupFunction.Median(Temperature.class);
+        GroupFunction function = new QuantityTypeArithmeticGroupFunction.Median(Temperature.class, null);
         State state = function.calculate(items);
 
         assertEquals(state.getClass(), expected.getClass());
         if (expected instanceof QuantityType expectedQuantityType) {
-            assertEquals(((QuantityType) state).getUnit(), expectedQuantityType.getUnit());
-            assertThat(((QuantityType) state).doubleValue(), is(closeTo(expectedQuantityType.doubleValue(), 0.01d)));
+            QuantityType stateQuantityType = ((QuantityType) state).toInvertibleUnit(expectedQuantityType.getUnit());
+            assertThat(stateQuantityType.doubleValue(), is(closeTo(expectedQuantityType.doubleValue(), 0.01d)));
         }
     }