First Iteration – Remove Bad Practices
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.
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
// 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
// initialize the api.global space in a separate Global element api.retainGlobal = true // initialize field for error handling api.local.errors = []
// 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
api.retainGlobal = true
We moved api.retainGlobal = true
to its own element called 'Global'.
api.local.errors = []
We initialized a local field where we will store any errors during the line item’s calculation.
api.local.countries = countries
We stored the result in api.local
for a further re-use in the CountryAdjustedPrice element.
Make sure you know when to use api.global and api.local.
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.
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?
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:
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.
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
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
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
def validRecords = data.findAll{ it.attribute4 <= targetDate }
We replaced the for-loop with a simple findAll
closure, making the code more readable.
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.
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
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
// 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
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.
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.
Notice the special syntax Filter.equal("ProductMaster__attribute2", pg)
. You can reference the related product from the Product Master and filter its attributes using this special syntax. It works in a similar way with the Customer Master and CX as well.
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.
if (!costs) { api.local.errors << "Missing costs for group $pg" return null }
Again, we added some validation. This time no visual indication is made.
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
def productCost = api.getElement("ProductCost") def productGroupAverageCost = api.getElement("ProductGroupAverageCost") return (productCost && productGroupAverageCost) ? (productCost - productGroupAverageCost) / productGroupAverageCost : null
After
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
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
.
The Elvis operator followed by ?: 0.0
will cause the next if-statement to fail even when the cost is 0.0 because of the Groovy truth. Make sure it is what you want. There may be cases when you want to specifically check for null.
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
def pg = api.product("Product Group") def marginAdj = api.vLookup("MarginAdj", "Margin Adj", pg) if (marginAdj == null) { marginAdj = 0.0 } return marginAdj
After
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
def cost = api.getElement("ProductCost") def marginAdj = api.getElement("MarginAdjustment") if (!cost) { return } if (!marginAdj) { return cost } return marginAdj ? cost * (1 + marginAdj) : cost
After
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
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
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
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.
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.
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.
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. |
Found an issue in documentation? Write to us.