Reliable ? Informative ? The quality of omni-automation examples

@Sal argues in another thread that new omni-automation examples need to go on using bear-trap and deprecated idioms like var and new Array() in JS for ‘uniformity’ with earlier examples supplied for OmniGraffle and OmniOutliner.

Presumably this means uniform quality rather than consistently leading customers into time-wasting and redundant complexity ?

Lets look at https://www.omni-automation.com/omnigraffle/graphics-00.html

The first example of var was discussed in another thread.

TL;DR

It:

  • Shows a strangely complicated way of doing something simple
  • shows no result when it is pasted and entered
  • pollutes the global name-space
  • Cat-walks several deprecated idioms which increase complexity and risk without reward
  • fails to show much simpler idioms using built-in Array methods like .map, .flatMap and .flat()
var gs = new Array()
canvases.forEach(function(cnvs){
    gs = gs.concat(cnvs.graphics)
})

Any one of these much simpler forms could have been directly pasted and (unlike the code above) actually shown a result:

  • canvases.flatMap(canvas => canvas.graphics)

or equivalently:

  • canvases.map(canvas => canvas.graphics).flat()

or (if some mysterious complexity is really insisted on):

  • [].concat(...canvases.map(canvas => canvas.graphics))

But apparently the author of the omni-automation examples has not looked at the standard built-in Array methods …

¯\_(ツ)_/¯


Do things get better ?

Not really. Let’s look further down the page to the first announcement that "here’s how to XYZ:"

And here’s how to reorder graphics in a specified layer.
Order Graphics in Specified Layer

var cnvs = document.windows[0].selection.canvas
var layerNames = cnvs.layers.map(function(layer){
    return layer.name
})
var targetName = "Cities"
if (layerNames.includes(targetName)){
    var targetLayer = cnvs.layers.filter(function(layer){
        if (layer.name === targetName){return layer}
    })
} else {
    throw new Error("Layer not found.")
}
targetLayer = targetLayer[0]
var names = targetLayer.graphics.map(function(graphic){
    return graphic.name
})
names.sort().reverse().forEach(function(name){
    graphic = cnvs.graphicWithName(name)
    graphic.orderAbove(targetLayer.graphics[0])
})

We’re used to the dense and exuberant cat-walking of deprecated practice, so let’s ignore that for the moment.

It get’s weirdly complex as soon as it "shows us how to" (tm) find a particular layer by its name.

var targetName = "Cities"
if (layerNames.includes(targetName)){
    var targetLayer = cnvs.layers.filter(function(layer){
        if (layer.name === targetName){return layer}
    })
} else {
    throw new Error("Layer not found.")
}

The only explanation for this strange complexity that comes to mind is that the author may not be familiar with the the very basic built-in Array.findIndex

Assuming that the name canvas has already been given to:

document.windows[0].selection.canvas

We could have skipped all that and simply written the elementary:

const layerIndex = canvas.layers.findIndex(
    layer => layer.name === "Cities"
)

If no matching layer is found we get the special index -1, and we can report that the layer was missing.

Otherwise, we can now directly refer to the layer we want:

canvas.layers[layerIndex]

I’m not sure that the muddle that was shown instead of this was very respectful of customer time, or very curious about learning the basics and passing them on.

But we’re not yet out of the woods with this example:

  1. it doesn’t work,
  2. it’s completely muddled (and misleading) about how Array.filter actually works, and
  3. it’s purpose is more than a little unclear.
    • We are sorting by name ? Most graphics don’t have a name by default …
    • We are sorting by graphic type ? Well … most shapes will share a type name or constructor name with many peers …
    • We are sorting by city names in the text of the shapes ? Then the code is sorting by the wrong shape property …

Try it.

2 Likes