Forum Discussion

Stuart_Weenig's avatar
Stuart_Weenig
Icon for Mastermind rankMastermind
2 months ago

What not to do when developing code

FYI, this shows a level of ignorance when it comes to troubleshooting and usability of return codes.

if (!organization) {
    println "Organization ID missing; device property meraki.api.org must be set."
    return 1
}
if (!network) {
    println "Network ID missing; device property meraki.api.network must be set."
    return 1
}
if (!serial) {
    println "Serial missing; device property auto.endpoint.serial_number or meraki.serial must be set."
    return 1
}
if (!productType) {
    println "Serial missing; device property auto.meraki.productype or meraki.productType must be set."
    return 1
}

Each different return statement could have a different non-zero return code (yes, it's an integer!). By returning a different value for each problem, the troubleshooter can identify exactly what the issue is just by looking at the return code. This shows that the developer possibly is laboring under the delusion that a return code can only be 0 or 1. 

While we're at it, this also evidences a lack of coding performance knowledge:

def category = it.category
def clientDescription = it.clientDescription
def clientId = it.clientId
def clientMac = it.clientMac
def description = it.description
def deviceName = it.deviceName
def deviceSerial = it.deviceSerial
def eventData = it.eventData.toString()
def networkId = it.networkId
def occurredAt = it.occurredAt
def type = it.type
def events = [:]
events.put("message", description)
events.put("category", category)
events.put("clientDescription", clientDescription)
events.put("clientId", clientId)
events.put("clientMac", clientMac)
events.put("description", description)
events.put("deviceName", deviceName)
events.put("deviceSerial", deviceSerial)
events.put("eventData", eventData)
events.put("networkId", networkId)
events.put("occurredAt", occurredAt)
events.put("type", type)

There's no point in defining a variable and storing it in memory to only use it once. If you're going to use it twice, it makes sense, but only barely. You're just wasting memory. Instead, do this:

def events = [
    "message":           it.description,
    "category":          it.category,
    "clientDescription": it.clientDescription,
    "clientId":          it.clientId,
    "clientMac":         it.clientMac,
    "description":       it.description,
    "deviceName":        it.deviceName,
    "deviceSerial":      it.deviceSerial,
    "eventData":         it.eventData.toString(,)
    "networkId":         it.networkId,
    "occurredAt":        it.occurredAt,
    "type":              it.type
]