Alphabetizing the layers in Adobe Illustrator

I have a script that alphabetizes the layers in Adobe Illustrator. It does so by sorting the layers in an array (using string names) and then using that to change the item order position (zOrder) of the layers. The script runs fine, however if there is a very large layer (which I deal with a lot) of over 8000+ it gets very sluggish and slow (over 10 minutes to sort). I would like a way to optimize the code to run faster, possibly a faster sort or search algorithm (it searches the array at one point which is the main source of the problem).

```
#target illustrator

/**
* sort all layers (and sublayers)
*/
if (app.documents.length > 0) {
var doc = app.activeDocument;
var docLayers = doc.layers;
var allLayers = [];
var currentLayers = [];

start();

function start() {
try {
recurseLayers ( docLayers );

var result = currentLayers.sort( function(a,b) { return a > b } );
sortLayer(allLayers, result);
alert("Done sorting!");
} catch (error){
logger(error);
}
}

function addLayer(currentLayer) {
var layerName;

if (currentLayer.typename == 'TextFrame') {
layerName = currentLayer.contents;
} else {
layerName = currentLayer.name;
}

currentLayers.push(layerName);
allLayers.push(currentLayer);

return currentLayers;
}

function sortLayer (obj, array) {
try {
var length = array.length;

for (var i=length; i--;) {
var name = array[i];
var item = search(obj, name);

if (item != -1) {
item.zOrder( ZOrderMethod.BRINGTOFRONT);
}
}
} catch(e) {
logger(e);
}
}

// Recursive loop to search all layers in active document
function recurseLayers ( layers ) {
var length = layers.length;
var currentLayer ;

What you are finding here is that one data structure does not fit all use cases. While it certainly makes sense to utilize an array to perform the sort (since you end up having to compare all the values anyway), an array certainly does not make sense for performing a lookup against any given layer name.

I would suggest that rather than using an array for your allLayers store of layer objects, that you build a key-value store - a hashmap - to allow for O(1) lookup of any given layer. In javascript this would be modeled using an object that might look like this:

{
    'alayer': {// layer object},
    'blayer': {// layer object},
    ...
    'zzz': {// layer object}
}

This object would not represent order of layers, as technically object properties have no order, so you would still need to maintain your currentLayers array to maintain ordering.

For convenience (and to prevent O(n) seeks against the ordered array), you might also want to store the value of the current order of the object in the array on the each layer object in the key-value store

For example:

{
    'alayer': {
        sortIndex: 0,
        ...
    },
    'blayer': {
        sortIndex: 1,
        ...
    },
    ...
}

To present this in terms of operational complexity (Big O). Your current methodlogy does this (here n would represent number of layers)

recursion and building of data structures - O(n)
layer name sort - O(n log n) average, O(n^2) worst case
applying the sort - O(n) for iteration, O(n) for lookup, thus O(n^2) worst case

Overall worst case = O((2 * n^2) + n)

With key-value store for layer objects:

recursion and building of data structures - O(n)
layer name sort - O(n log n) average, O(n^2) worst case
applying the sort - O(n)

Overall worst case = O(n^2 + 2n)

I would recommend getting into the habit of using exact comparisons (===, !==) instead of loose comparisons (==, !=) as your default means of comparison. This can help prevent against unexpected comparison behavior. I would go so far as to say that loose comparisons should ONLY be used in cases which truly warrant it and should ideally be accompanied by a comment explaining why the loose comparison is appropriate for that case.

var doc = app.activeDocument;
var docLayers = doc.layers;

Not sure that you are getting any value for storing duplicate copies of this information in this scope. You are only using app.activeDocument.layers a single time, in your start() function. Why not have first line of that function simply be recurseLayers(app.activeDocument.layers)?

var currentLayers = [];

This is a really bad variable name. As I was reading through this code I was thinking that this array stored layer objects, when in fact, it stores only the layer names. I have no idea what current portion of the variable name would be referring to either. Be specific and meaningful in your variable, function, etc. naming. Here layerNames would probably be better.

var result = currentLayers.sort( function(a,b) { return a > b } );

sort() sorts an array in place, so no need to assign to new variable. No need to define a function here as you are just using default sort behavior. This line should probably just be:

currentLayers.sort();

function addLayer(currentLayer) {
    var layerName = currentLayer.name;
    if (currentLayer.typename == 'TextFrame') {
            layerName = currentLayer.contents;
    }

    currentLayers.push(layerName);
    allLayers.push(currentLayer);

    return currentLayers;
}

Why return the array of names from this function?

If using key-value store this function might look like:

function addLayer(currentLayer) {
    var layerName = currentLayer.name;
    if (currentLayer.typename == 'TextFrame') {
            layerName = currentLayer.contents;
    currentLayers.push(layerName);
    // assumes allLayers is name of key-value store
    allLayers[layerName] = currentLayer;
}

Note that this eliminates need for a search() function.

function sortLayer (obj, array) {

Bad function name. There is no sorting happening. Here you are actually applying the previously determined sort to the layers. Perhaps applySortToLayers or similar would be better name. Why do you need to pass any parameters to this method at all? You already have necessary data available to this function scope.

If using a key-value store for your layer objects, this function might look like:

function applySortToLayers() {
    currentLayers.forEach(function(name, i) {
        // assumes allLayers is key-value store for layers
        // also assumes that there is SENDTOBACK opposite of BRINGTOFRONT
        // if not, you would need to iterate layerNames in reverse as
        // in your current code
        allLayers[name].zOrder( ZOrderMethod.SENDTOBACK);
        // if you want to add reference to sort order
        allLayers[name].sortIndex = i;
    });
}

StackExchange Code Review Q#153590, answer score: 4