1
\$\begingroup\$

I am writing a conditional that checks two values, for several different scenarios.

What I am modifying is a series of objects, some of these objects are in arrays, and some are just arrays. I need to check these values and then check a file, and then check my values (the series of objects, array objects, and arrays) and then "Update" the old data with my new data if needed. However the user has the ability to turn some sections off if they don't want their data to be overwritten in the case of changes. So in order to do this I check if a certain property exists (the ones used to turn them on or off) and then check if its on or if its off. which has led to a conditional of this size: (And this is NOT everything, I stopped here to see if there was a shorter less counter-productive way of doing this

if ((this.Config[key] === this.Config.PunishTiers || this.Config[key] === this.Config.Titles || this.Config[key] === this.Config.Ranks || this.Config[key] === this.Config.Messages && this.Config.Message.Messages) || (this.Config[key] === this.Config.Prefix && this.Config.Prefix.Prefixes))
             || (this.Config[key] === this.Config.Settings && this.Config.Settings.Settings)) {
                oldData.push(this.Config[key]);
        }

The first three checks do not have the setting, so they are fine but everything after has the check if it exists, and check for the setting to be on or off. Is there any way to reduce this? Make it neater and possibly less of a struggle when used.

This has to stick with mainly pure JavaScript if possible.

This is being used through a plugin for the oxide framework. To check a plugin's configuration file grab the data, check for local configs and apply changes where specified. or "Updates"

Here is the whole function setup:

loadSmartConfig: function(changed) {
            print("Smart Updating Data Now.")
            var oldData = [],
                newData = [];
            for (var key in this.Config) {
                if (this.Config[key] === this.Config.PunishTiers || this.Config[key] === this.Config.Titles || this.Config[key] === this.Config.Ranks) {
                    oldData.push(this.Config[key]);
                if ((this.Config[key] === this.Config.Messages && this.Config.Message.Messages) 
                    || (this.Config[key] === this.Config.Prefix && this.Config.Prefix.Prefixes)
                    || (this.Config[key] === this.Config.Settings && this.Config.Settings.Settings)) {
                    oldData.push(this.Config[key]);
                }
                } else {
                    continue;


}
            }
            if (oldData.length > 0) {
                this.LoadDefaultConfig();
            }

        if (this.Config.PunishTiers !== undefined && this.Config.Titles !== undefined && this.Config.Ranks !== undefined) {
            for (var i = 0; i < oldData.length; i++) {
                if (oldData[i].length === 1) {
                    this.Config.PunishTiers = oldData[i];
                } else if (oldData[i].hasOwnProperty("authLvl")) {
                    this.Config.Titles = oldData[i];
                } else if (oldData[i].hasOwnProperty("rank")) {
                    this.Config.Ranks = oldData[i];
                } else if (oldData[i].hasOwnProperty("Messages") && oldData[i].Messages) {
                    this.Config.Messages = oldData[i];
                } else if (oldData[i].hasOwnProperty("Prefixes") && oldData[i].Prefixes) {
                    this.Config.Prefix = oldData[i];
                } else if (oldData[i].hasOwnProperty("Settings") && oldData[i].Settings){
                    this.Config.Settings = oldData[i];
                }
            }
            this.SaveConfig();
        },

this is also why I was pushing for pure JavaScript if possible.

\$\endgroup\$
2
\$\begingroup\$

If your main concern is making it more human-readable, one approach may be to use the bitwise OR assignment operator |= like so:

var pushRequired = false;
pushRequired |= this.Config[key] === this.Config.PunishTiers;
pushRequired |= this.Config[key] === this.Config.Titles;
pushRequired |= this.Config[key] === this.Config.Ranks;
pushRequired |= this.Config[key] === this.Config.Messages && this.Config.Message.Messages;
pushRequired |= this.Config[key] === this.Config.Prefix && this.Config.Prefix.Prefixes;
pushRequired |= this.Config[key] === this.Config.Settings && this.Config.Settings.Settings;
if(pushRequired){
    oldData.push(this.Config[key]);
}

The downsides of this approach are purely performance-related:

  • it uses an extra bit for the pushRequired variable
  • it's not only comparing but also making an assignment with each comparison
  • bitwise OR assignment will not have short circuiting behavior, so it'll keep performing comparisons even if the value is already true

It's also worth noting that using bitwise OR assignment you end up with a number rather than a Boolean; it'll be 0 or 1 in this case, which JavaScript nicely interprets as false or true, respectively.

With that said, you could accomplish something visually similar by just breaking up your comparison into its components with line breaks.

In your example code, I see no reason to have two different IF statements leading to oldData.push(); the conditions could be combined like so:

if ((this.Config[key] === this.Config.PunishTiers) || 
    (this.Config[key] === this.Config.Titles) || 
    (this.Config[key] === this.Config.Ranks) ||
    (this.Config[key] === this.Config.Messages && this.Config.Message.Messages) || 
    (this.Config[key] === this.Config.Prefix && this.Config.Prefix.Prefixes) ||
    (this.Config[key] === this.Config.Settings && this.Config.Settings.Settings)) {
    oldData.push(this.Config[key]);
}

Note that I added some unnecessary parentheses just to keep it visually consistent.

\$\endgroup\$
  • \$\begingroup\$ Bitwise might be something I'll look into, thank you for your advise it was more along the lines of keeping it clean and efficient in a sense really. \$\endgroup\$ – dhershman Mar 14 '15 at 3:13
2
\$\begingroup\$

It's hard to analyze without the whole context, but a possible workaround could be to add the whole keys in a collection, and apply a filter like _.some of underscore.js.

var aColection = [this.Config.Ranks, this.Config.Message,..]
_.some(aCollection, function (element) {
  return this.Config[key] === element
})

If you don't want add this library to your core code, you can simply copy the definition of some.

GitHub

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.