setCliChanged simplified logic adds redundant string compares

[Ryan-DowlingSoka]

Hi! Thanks for pulling #89. I reviewed the changes since then. They all look pretty good. The only one I wanted to bring up as a potential issue is 29aecdd which moves the string comparison language == "Windows" etc inside the for loop. Although this looks simpler and cleaner, it does mean that the string compare is done each time instead of done once at the start and moved each other time.

I'm not familiar with the internals of the javascript interpreter, but I'd be shocked to learn that this is a more efficient algorithm. I guess there is a chance that the extra bytes for the few extra lines outweigh the algorithm costs in download, but that feels more dependent on internet speeds etc.

I think these string compares and conditionals pulled back out of the loop for algorithm simplicity, preventing redundant operations in the loops.

[Ryan-DowlingSoka]

@dlemstra for vis.

[dlemstra]

That's a great point and thanks for reviewing my changes. Next time feel free to directly open a PR that addresses this. I can also make the changes myself if you would prefer me to do that.

[Ryan-DowlingSoka]

That'll be easier for me this time if you could make the change. :) I'm in a bit of a hybrid workstation setup right now, so a lot of this is being done from mobile. 🤢 But in the future I can just do PRs. 👍

[dlemstra]

It seems that the minifier that I am using (https://minify-js.com/) moves the compare inside the for loop so I will keep it like this.

[Ryan-DowlingSoka]

@dlemstra are you sure? I copied that segment into that minifier's website and it did not move the compare inside the forloop.

input:

function setCliLanguage(language){
    if (imagemagick_cli_lists == null) {
      imagemagick_cli_lists = createCliLists();
    }

    let display_windows = language == "Windows" ? "inherit" : "none";
    let display_linux = language == "Linux" ? "inherit" : "none";
    let display_batch = language == "Batch File" ? "inherit" : "none";
    for (let node of imagemagick_cli_lists["windows"]) {
      node.style.display = display_windows;
    }
    for (let node of imagemagick_cli_lists["linux"]) {
      node.style.display = display_linux;
    }
    for (let node of imagemagick_cli_lists["batch"]) {
      node.style.display = display_batch;
    }
    for (let node of imagemagick_cli_lists["select"] ) {
      node.value = language;
    }
  }

output: (formatted version of minified code):

function setCliLanguage(i) {
   null == imagemagick_cli_lists && (imagemagick_cli_lists = createCliLists());
   let l = "Windows" == i ? "inherit" : "none",
       e = "Linux" == i ? "inherit" : "none",
       s = "Batch File" == i ? "inherit" : "none";
   for (let i of imagemagick_cli_lists.windows) i.style.display = l;
   for (let i of imagemagick_cli_lists.linux) i.style.display = e;
   for (let i of imagemagick_cli_lists.batch) i.style.display = s;
   for (let l of imagemagick_cli_lists.select) l.value = i
}

[dlemstra]

Yeah you are right... weird. I probably went to the output tab without re-compressing again...