3
\$\begingroup\$

I've been trying to learn Groovy lately and I tried to solve a problem which involves designing a routine that will calculate the average Product price per Group.

I have the following data:

// contains information about [Product, Group, Cost]
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]
]

// contains information about Category classification based on product Cost
// [Category, Cost range from (inclusive), Cost range to (exclusive)]
// i.e. if a Product has Cost between 0 and 25, it belongs to category C1
category = [
    ["C3", 50, 75],
    ["C4", 75, 100],
    ["C2", 25, 50],
    ["C5", 100, null],
    ["C1", 0, 25]
]

// contains information about margins for each product Category
// [Category, Margin (either percentage or absolute value)]
margins = [
    "C1" : "20%",
    "C2" : "30%",
    "C3" : "0.4",
    "C4" : "50%",
    "C5" : "0.6"]

What I tried so far is the following (I mixed some Java here):

def calculateResult() {
    def group1 = []
    def group2 = []
    def group3 = []
    def pricePerGroupMap = [:]

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

    if (max == null) {
        max = 9999.9;
    }

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

            if (selectedMarginAsString.endsWith("%")) {
                selMargin = new BigDecimal(selectedMarginAsString.trim().replace("%", "")).divide(BigDecimal.valueOf(100));
            } else {
                selMargin = new BigDecimal(selectedMarginAsString);
            }

            if (product[1].equals("G1")) {
                group1.add(product[2] * (1 + selMargin))
            } else if (product[1].equals("G2")) {
                group2.add(product[2] * (1 + selMargin))
            } else if (product[1].equals("G3")) {
                group3.add(product[2] * (1 + selMargin))
            }
        }

    }
}
pricePerGroupMap["G1"] = group1.sum() / group1.size()
pricePerGroupMap["G2"] = group2.sum()/ group2.size()
pricePerGroupMap["G3"] = group3.sum()/ group3.size()
print pricePerGroupMap
}

I am sure there are better ways to optimize this algorithm taking advantage of closures.

\$\endgroup\$
1
\$\begingroup\$

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
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Hi Marcelo! Can you edit your post to provide some more detail about what makes this answer better than the original code posted in the question? \$\endgroup\$ – cariehl May 9 at 17:05
  • 1
    \$\begingroup\$ Sure @cariehl, thanks for the heads up. I added more detail on the changes I made and how I implemented them. Please, let me know if more details are needed for this post. Thanks in advance. \$\endgroup\$ – Marcelo Tataje May 9 at 20:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.