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

Global
// initialize the api.global space in a separate Global element
api.retainGlobal = true

// initialize field for error handling
api.local.errors = []
Country
// 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.


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

(tick) 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

ProductCost
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

ProductCost
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

ProductGroupAverageCost
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

ProductGroupAverageCost
// 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

AverageCostDiffPct
def productCost = api.getElement("ProductCost")
def productGroupAverageCost = api.getElement("ProductGroupAverageCost")

return (productCost && productGroupAverageCost) ? (productCost - productGroupAverageCost) / productGroupAverageCost : null

After

AverageCostDiffPct
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

MarginAdjustment
def pg = api.product("Product Group")

def marginAdj = api.vLookup("MarginAdj", "Margin Adj", pg)
if (marginAdj == null) {
  marginAdj = 0.0
}

return marginAdj

After

MarginAdjustment
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

BasePrice
def cost = api.getElement("ProductCost")
def marginAdj = api.getElement("MarginAdjustment")
if (!cost) {
  return
}

if (!marginAdj) {
  return cost
}

return marginAdj ? cost * (1 + marginAdj) : cost

After

BasePrice
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

CountryAdjustedPrice
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

CountryAdjustedPrice
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 api.retainGlobal and initialize the api.local.errors field.

Country

We fetched the countries from the CountryFactor PP and stored the results in the api.local space for a re-use in the CountryAdjustedPrice element.

AbortIfInputGeneration

We added this element with the statement api.abortCalculation() if the execution runs in the input generation mode. Therefore no other element will be calculated after this element in the input generation mode.

ProductCost

We added some validation and error handling code.

ProductGroupAverageCost

We showed how to use api.stream instead of api.find and showed how to use an advanced filter on the Product Master attributes.

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.