Groovy script to get average price per group from different data arrays

This is already old and your coding skills have probably improved enough since then that you no longer need a review, but I just couldn't resist revising the code. May it help others appreciate Groovy.

Main changes:

• introduced two classes Product and Category for better readability and transformed the lists accordingly. The annotation @Canonical provides cool functionality like a constructor and a meaningful toString() method.
• completely changed the calculateResult.

Method calculateResult

• Within the method, it makes much more sense to first group the products by their groups and then iterate over their products per group than to start iterating by categories.
• I put the categories into a TreeMap to find the right Category for a Product. This may seem a bit over the top, but it not only improves the time complexity for one lookup of a Category for a given Product from O(n) to O(log(n)), where n is the number of categories, but also is very convenient to use here. The floorEntry method on a TreeMap

returns a key-value mapping associated with the greatest key less than or equal to the given key, or null if there is no such key

• The method average() came with Groovy 3.

Codestyle

• If you don't use def or the type before the variable name when declaring variables, the variable will be added to the script's binding. It is more readable and better for encapsulation to define them the usual way and then pass them to the method explicitly.
• Although Groovy ignores generics in Maps and Lists, I'd advise including them for documentation purposes if it's not obvious. Your colleagues and your future self will appreciate that. In addition, you help your IDE to support you.

import groovy.transform.Canonical

List<Product> products = [
    ["A", "G1", 20.1],
    ["B", "G2", 98.4],
    ["C", "G1", 49.7],
    ["D", "G3", 35.8],
    ["E", "G3", 105.5],
    ["F", "G1", 55.2],
    ["G", "G1", 12.7],
    ["H", "G3", 88.6],
    ["I", "G1", 5.2],
    ["J", "G2", 72.4]
].collect { new Product(*it) }

Map<String, String> margins = [
    "C1" : "20%",
    "C2" : "30%",
    "C3" : "0.4",
    "C4" : "50%",
    "C5" : "0.6"
]

List<Category> categories = [
        ["C3", 50, 75],
        ["C4", 75, 100],
        ["C2", 25, 50],
        ["C5", 100, null],
        ["C1", 0, 25]
].collect{ new Category(*it, Category.parseMargin(margins[it[0]])) }

Map<String, BigDecimal> avgPricePerGroupMap = calculateResult(products, categories)
assert avgPricePerGroupMap == [G1:37.498, G2:124.48, G3:116.08]

Map<String, BigDecimal> calculateResult(List<Product> products, List<Category> categories) {
    TreeMap<Integer, Category> cats = new TreeMap(categories.collectEntries { Category cat -> [cat.minPrice, cat] })

    products.groupBy { Product p -> p.group }
            .collectEntries { String group, List<Product> ps ->
                List<BigDecimal> prices = ps.collect { Product p -> p.price * (1 + cats.floorEntry(p.price).value.margin) }
                [group, prices.average()]
            }
}

@Canonical
class Product {
    String name
    String group
    BigDecimal price
}

@Canonical
class Category {
    String id
    BigDecimal minPrice
    BigDecimal maxPrice
    BigDecimal margin

    static BigDecimal parseMargin(String margin){
        return margin.endsWith("%") ? margin[0..-2].toBigDecimal() / 100 : margin.toBigDecimal()
    }
}
```

After being investigating a little bit more about usage of closures and some other facilities that Groovy provides, I managed to reduce a little bit the algorithm, after looking for benchmarking posts, I believe this is probably not the most optimal solution, but it looks a lot better than the first proposal (the one in my question).

One of the things I considered to improve the solution is to use Groovy "maps" instead of multiple arrays to hold the data.

I also considered using one additional variable to hold the value of a group being evaluated.

And finally, I decided to avoid using more conditionals to add the results to the arrays and use a more simple instruction taking advantage of the map definition.

MAX_DEFAULT_VALUE_WHEN_NULL = 99999.9

def calculateResult() {
    def pricesPerGroupMap = [G1:[], G2:[], G3:[]]
    def avgPricePerGroupMap = [:]

    category.each { cat ->
        String categoryId = cat[0]
        BigDecimal min = cat[1]
        BigDecimal max = cat[2]

        if (max == null) {
            max = MAX_DEFAULT_VALUE_WHEN_NULL
        }

        products.each { product ->
            BigDecimal currProductPrice = BigDecimal.valueOf(product[2])
            String group = product[1]
            if (currProductPrice.compareTo(min) >= 0 && currProductPrice.compareTo(max) == -1) {
                String selectedMarginAsString = margins.getAt(categoryId);
                BigDecimal selMargin = 0.0;

                if (selectedMarginAsString.endsWith("%")) {
                    selMargin = new BigDecimal(selectedMarginAsString.trim().replace("%", "")).divide(BigDecimal.valueOf(100))
                } else {
                    selMargin = new BigDecimal(selectedMarginAsString)
                }
                pricesPerGroupMap[group].add(currProductPrice * (1 + selMargin))
            }
        }
    }
    pricesPerGroupMap.each { k, v -> 
        avgPricePerGroupMap.put(k, new BigDecimal(v.sum()).divide(v.size(), 1, RoundingMode.UP))
    }
    return avgPricePerGroupMap
}