The goal of the first iteration is to remove some of the bad practices and make the code more readable.
In the following examples the original element is on the left side and on the right side is the final result of the refactoring.
Table of Contents | ||
---|---|---|
|
Global Element, Input & Data Validation, Groovy Closures
Let’s focus on the following best practices:
- The statement api.retainGlobal should be the first statement in a dedicated first element of the logic as described in /wiki/spaces/KB/pages/313098544.
- User inputs and data should be validated.
- Groovy Closures can make the code more readable.
Country Element
Let’s have a look at the Country element and what we can do with it to better follow the best practices. We can definitely create the Global element, use a Groovy closure to add sorting and add some user input validation.
Before
Code Block |
---|
// initialize the api.global space api.retainGlobal = true // fetch the country factors def countries = api.findLookupTableValues("CountryFactor") // transform the countries into a map of Country Code : Description // so we can use it as labels for the drop-down def labels = [:] for (row in countries) { labels[row.key1] = row.attribute2 } def options = labels.keySet().toList() // create the user entry return api.option("Country", options, labels) |
After
Code Block | ||
---|---|---|
| ||
// initialize the api.global space in a separate Global element api.retainGlobal = true // initialize field for error handling api.local.errors = [] |
Code Block | ||
---|---|---|
| ||
// fetch the country factors def countries = api.findLookupTableValues("CountryFactor") // store the factors in the api.local space for further re-use api.local.countries = countries // transform the countries in a map so we can use it as labels for the drop-down // and sort the results by the label alphabetically def labels = countries.collectEntries { [it.key1, it.attribute2] }.sort { it.getValue() } options = labels.keySet().toList() // create the user entry def selectedCountry = api.option("Country", options, labels) // add input validation and set a default value if the input is not specified if (!selectedCountry) { api.yellowAlert("Country not selected, defaulting to AU") api.local.errors << "Country not selected, defaulting to AU" selectedCountry = "AU" } return selectedCountry |
Step-by-step Analysis
Code Block |
---|
api.retainGlobal = true |
We moved api.retainGlobal = true
to its own element called 'Global'.
Code Block |
---|
api.local.errors = [] |
We initialized a local field where we will store any errors during the line item’s calculation.
Code Block |
---|
api.local.countries = countries |
We stored the result in api.local
for a further re-use in the CountryAdjustedPrice element.
Info |
---|
Make sure you know when to use api.global and api.local. |
Code Block |
---|
def labels = countries.collectEntries { [it.key1, it.attribute2] }.sort { it.getValue() } |
We used a Groovy closure collectEntries
to transform the records into a map. Despite the fact we are returning an array, the first element is a key and the second is a value of the map entry. We can also return the entry as [(it.key1) : it.attribute2]
.
It is a good practice to put each closure body on a new line for better readability.
Code Block |
---|
if (!selectedCountry) { api.yellowAlert("Country not selected, defaulting to AU") api.local.errors << "Country not selected, defaulting to AU" selectedCountry = "AU" } |
We added a validation and show a yellow alert as a visual indication if anything goes wrong. We also want to show the error at the end of the calculation and set the default country to “AU”.
In more severe cases you can also consider calling api.throwException
to completely stop the execution of the logic. Depending on the error handling mode of the price list, the calculation will either abort or continue.
Removing Input Generation Condition from Every Element
In release 10.0 Bees Knees, api.isInputGenerationExecution()
replaces api.isSyntaxCheck()
. In older versions isInputGenerationExecution
is not supported.
Are you also familiar with the bad practice of putting the so-called input generation if-statement in every element like this?
Code Block |
---|
if (api.isInputGenerationExecution()) { return } // ... the rest of the element |
Let’s solve it according to the documented best practice /wiki/spaces/KB/pages/784957624.
We will remove the if-statement from all the elements and add one extra element right AFTER the Country element called AbortIfInputGeneration with the following code:
Code Block |
---|
if (api.isInputGenerationExecution()) { // the calculation won't continue after this element in input generation mode // note: the rest of this element is executed as usual api.abortCalculation() } |
The call to api.abortCalculation()
will make sure the rest of the logic will not be executed in the input generation mode, so we don’t have to check it every time in the following elements.
Note |
---|
Make sure you initialize all the user inputs BEFORE this element. Otherwise they will not be generated because they are created only in the input generation mode. In our case we initialize the user input in the Country element, that is why we add the AbortIfInputGeneration element right after it. |
Product Cost Element
Let’s have a look at the product cost calculation. Here we can use a closure to make the search of the valid row more readable and also add some validation.
Before
Code Block | ||
---|---|---|
| ||
def sku = api.product("sku") // get the current or target date def targetDate = api.calendar().format("yyyy-MM-dd") // fetch the costs def data = api.productExtension("Product_Costs") // find only records where the validity date is lower than the target date def validRecords = [] for (row in data) { if (row.attribute4 <= targetDate) { validRecords << row } } if (!validRecords) { return null } // sort the costs by date in descending order and pick the most recent one validRecords = validRecords.sort { a, b -> return (b.attribute4 == null ? a.attribute4 == null : b.attribute4.compareTo(a.attribute4)); } return validRecords[0].attribute1.toBigDecimal() |
After
Code Block | ||
---|---|---|
| ||
def sku = api.product("sku") // get the current or target date def targetDate = api.calendar().format("yyyy-MM-dd") // fetch the costs def data = api.productExtension("Product_Costs") // find only records where the validity date is lower than the target date def validRecords = data.findAll{ it.attribute4 <= targetDate } if (!validRecords) { api.redAlert("Cost not found for $sku") api.local.errors << "Cost not found for $sku" return null } // sort the costs by date in descending order and pick the most recent one validRecords = validRecords.sort { a, b -> return b.attribute4 <=> a.attribute4; } return validRecords[0].attribute1.toBigDecimal() |
Step-by-step Analysis
Code Block |
---|
def validRecords = data.findAll{ it.attribute4 <= targetDate } |
We replaced the for-loop with a simple findAll
closure, making the code more readable.
Code Block |
---|
if (!validRecords) { api.redAlert("Cost not found for $sku") api.local.errors << "Cost not found for $sku" return null } |
We added data validation. If the cost is not found, we show a red alert and also an error at the end of the calculation.
Code Block |
---|
validRecords = validRecords.sort { a, b -> return b.attribute4 <=> a.attribute4; } |
Here we simplified the code by using the Spaceship operator. Read more about it at Recommended Operators.
You probably noticed we could use the filter on Valid From directly in the api.productExtension
call. You’re right. We will do that later.
Find versus Stream, Advanced Filter
ProductGroupAverageCost Element
Here we show how to link a Groovy library and use an advanced filter together with data streaming.
Before
Code Block | ||
---|---|---|
| ||
def pg = api.product("Product Group") // fetch the costs from the PX def costs = api.find("PX", Filter.equal("name", "Product_Costs"), Filter.equal("ProductMaster__attribute2", pg)) // add the cost to the total sum if the product's group is the same group as the current product's def sum = 0 def count = 0 for (c in costs) { if (api.product("Product Group", c.sku) == pg) { api.trace("Adding sum of $c.sku", null, c.attribute1) sum += c.attribute1.toBigDecimal() ++count } } def avgCost = sum / count return libs.DemoLib.MathUtil.round(avgCost, 4) |
After
Code Block | ||
---|---|---|
| ||
// let's create a reference to the library so the code is better readable // especially useful when there are multiple libraries and/or long names def Math = libs.DemoLib.MathUtil def pg = api.product("Product Group") // fetch the costs from the PX // notice using the Product Master attribute to filter the PX def cit = api.stream("PX", "sku", Filter.equal("name", "Product_Costs"), // PX Name Filter.equal("ProductMaster__attribute2", pg)) // Product Group def costs = cit.collect{ it } cit.close() if (!costs) { api.local.errors << "Missing costs for group $pg" return null } def avgCost = costs.sum{ it.attribute1.toBigDecimal() } / costs.size() return Math.round(avgCost, 4) |
Step-by-step Analysis
Code Block |
---|
def Math = libs.DemoLib.MathUtil |
We created a reference to the library libs.DemoLib.MathUtil
, so we can later call just Math.round()
instead of having to type the whole thing. This is especially useful if the library is used multiple times within the same element.
Code Block |
---|
def cit = api.stream("PX", "sku", Filter.equal("name", "Product_Costs"), // PX Name Filter.equal("ProductMaster__attribute2", pg)) // Product Group def costs = cit.collect{ it } cit.close() |
We use api.stream
instead of api.find
. Using api.find
is wrong here because we can have more than 200 records (which is the default) or generally more than api.getMaxFindResultsLimit()
records.
Tip |
---|
Notice the special syntax |
Note |
---|
Never forget to close the stream! Imagine you leave it open and the logic is run over 100,000 items of a price list. That will leave 100,000 streams open which would be at least a performance issue. |
Code Block |
---|
if (!costs) { api.local.errors << "Missing costs for group $pg" return null } |
Again, we added some validation. This time no visual indication is made.
Code Block |
---|
def avgCost = costs.sum{ it.attribute1.toBigDecimal() } / costs.size() |
We used the sum
closure using the attribute1
converted to a BigDecimal number.
AverageCostDiffPct Element
Here we did just some simple changes for the sake of readability.
Before
Code Block | ||
---|---|---|
| ||
def productCost = api.getElement("ProductCost") def productGroupAverageCost = api.getElement("ProductGroupAverageCost") return (productCost && productGroupAverageCost) ? (productCost - productGroupAverageCost) / productGroupAverageCost : null |
After
Code Block | ||
---|---|---|
| ||
def cost = api.getElement("ProductCost") ?: 0.0 def groupAvgCost = api.getElement("ProductGroupAverageCost") if (cost && groupAvgCost) { return (cost - groupAvgCost) / groupAvgCost } return null |
Step-by-step Analysis
Code Block |
---|
def cost = api.getElement("ProductCost") ?: 0.0 |
We added the default value using the Elvis operator. This means when api.getElement(“ProductCost”)
returns a “false” value according to Groovy truth, it is replaced by 0.0
.
Note |
---|
The Elvis operator followed by |
Code Block |
---|
if (cost && groupAvgCost) { return (cost - groupAvgCost) / groupAvgCost } |
Next, we also shortened the names of the variables and removed the ternary operator for better readability. It is not always a win to use the fancy stuff. Readability has priority!
MarginAdjustment Element
Not much to improve here in this step. Let’s just shorten the statement by adding the Elvis operator ?:
.
Before
Code Block | ||
---|---|---|
| ||
def pg = api.product("Product Group") def marginAdj = api.vLookup("MarginAdj", "Margin Adj", pg) if (marginAdj == null) { marginAdj = 0.0 } return marginAdj |
After
Code Block | ||
---|---|---|
| ||
def pg = api.product("Product Group") def marginAdj = api.vLookup("MarginAdj", "Margin Adj", pg) ?: 0.0 return marginAdj |
BasePrice Element
Here we just shortened the statement using the ternary operator.
Before
Code Block | ||
---|---|---|
| ||
def cost = api.getElement("ProductCost") def marginAdj = api.getElement("MarginAdjustment") if (!cost) { return } if (!marginAdj) { return cost } return marginAdj ? cost * (1 + marginAdj) : cost |
After
Code Block | ||
---|---|---|
| ||
def cost = api.getElement("ProductCost") def marginAdj = api.getElement("MarginAdjustment") if (!cost) { return } return marginAdj ? cost * (1 + marginAdj) : cost |
CountryAdjustedPrice Element
In this element optimize a bit by reading the previously cached value.
Before
Code Block | ||
---|---|---|
| ||
def basePrice = api.getElement("BasePrice") if (!basePrice) { return } def selectedCountry = api.getElement("Country") def productGroup = api.product("Product Group") def factors = api.findLookupTableValues("CountryFactor", Filter.equal("key1", selectedCountry), Filter.equal("key2", productGroup)) def multiplier = factors ? factors[0].attribute1 : 1.0 if (multiplier == null) { multiplier = 1.0 } return basePrice * multiplier |
After
Code Block | ||
---|---|---|
| ||
def basePrice = api.getElement("BasePrice") if (!basePrice) { return } def selectedCountry = api.getElement("Country") def productGroup = api.product("Product Group") def countries = api.local.countries // find the multiplier for the correct country & product group combination def multiplier = countries.find { it.key1 == selectedCountry && it.key2 == productGroup }.attribute1 if (multiplier == null) { multiplier = 1.0 } return basePrice * multiplier |
Step-by-step Analysis
Code Block |
---|
def countries = api.local.countries |
Because we cached the records of the CountryFactor PP in the Country element and stored it within api.local
space, we do not need to load it again and save some valuable time.
Code Block |
---|
def multiplier = countries.find { it.key1 == selectedCountry && it.key2 == productGroup }.attribute1 |
Instead of accessing the database, we can use the find
closure to find the country factor with the matching country code and product group. If there were many countries, it would be even better to store them in a hash map with the selected country & product group as the key.
Code Block |
---|
if (multiplier == null) { multiplier = 1.0 } |
As zero can be a valid value, we cannot use the Groovy truth here because that would rule out zeros. Therefore we specifically check for null.
Error Handling
Errors Element
Let’s add the last element which displays the errors gathered throughout the logic execution and puts them in a neat matrix.
Code Block |
---|
if (api.local.errors) { def m = api.newMatrix("Number", "Message") api.local.errors.eachWithIndex { it, i -> m.addRow([ "Number" : i + 1, "Message": it ]) } return m } return null |
Notice the eachWithIndex
closure which is an equivalent of the classic for-loop.
Summary
The result after the first iteration is the following structure of the logic.
Element | Changes we made |
---|---|
Global | We created this element by splitting the original Country element. We call |
Country | We fetched the countries from the CountryFactor PP and stored the results in the |
AbortIfInputGeneration | We added this element with the statement |
ProductCost | We added some validation and error handling code. |
ProductGroupAverageCost | We showed how to use |
AverageCostDiffPct | We showed that using fancy stuff as the ternary operator is not always a win. |
MarginAdjustment | Here we just shortened a statement using the Elvis operator. |
BasePrice | Here we just shortened a statement using the ternary operator. |
CountryAdjustedPrice | We re-used the countries fetched in the Country element. |
Errors | Displays all errors in a nice matrix. |