3
\$\begingroup\$

I am making an application in which the user has the ability to add, remove, and edit JSON data through DOM interaction. I have created a working JavaScript-only prototype that accepts certain variables to manipulate certain elements of the JSON data, although it is rather messy and seems rather impossible for me to implement with the DOM.

Right now, my current structure is:

  • Example Modding Session
    • Modding Section
      • A mod
      • A mod
    • Another Modding Section
      • A mod
      • A mod
      • A mod
  • Another Modding Session
    • Modding Section
      • Another mod
      • Another mod
      • Another mod
    • Modding Section
      • Another mod

Which has worked fine for me, however, my functions are very repetitive, which I would like to fix.

var addSession = function(sessionName) {

    var selfSessionName = sessionName || undefined;
    if(!selfSessionName) return;

    moddingSessions.push({
        "name": selfSessionName,
        "sections": []
    });

}

Here you may start to see a pattern:

var addSection = function(sectionName, sessionName) {

    var selfSessionName = sessionName || undefined,
        selfSectionName = sectionName || undefined;
    if(!selfSectionName || !selfSectionName) return;

    for(var i = 0; i < moddingSessions.length; i++) {
        if(moddingSessions[i].name === selfSessionName) {
            moddingSessions[i].sections.push({
                "name": selfSectionName,
                "mods": []
            });
        }
    }

}

And here:

var addMod = function(modContent, sectionName, sessionName) {

    var selfSessionName = sessionName || undefined,
        selfSectionName = sectionName || undefined,
        selfModContent  = modContent  || undefined;
    if(!selfModContent || !selfSectionName || !selfSectionName) return;

    for(var i = 0; i < moddingSessions.length; i++) {
        if(moddingSessions[i].name === selfSessionName) {
            for(var j = 0; j < moddingSessions[i].sections.length; j++) {
                if(moddingSessions[i].sections[j].name === selfSectionName) {
                    moddingSessions[i].sections[j].mods.push(modContent);
                }
            }
        }
    }

}

As you can see, it works, but isn't very effective. I also have no idea how to handle each and every parameter when using this with DOM interaction. I hope there is a better way to achieve the same functionality.

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

I see several problems.

  1. Redeclaring function arguments as local variables
  2. Assigning variables undefined by default
  3. Failing silently when "falsey" arguments are passed

Redeclaring Function Arguments

This is absolutely unnecessary. You're only creating more variables that must be garbage collected by the browser, adding weight to your code and getting no tangible benefit from it. Now if you wanted copies of the arguments because you need to modify the data but still know the original values passed to the function, then you have a case for creating variables separate from the arguments, but this is an exception to the rule.

Assigning Variables undefined By Default

This is already how JavaScript works. This pattern should be used to give meaningful default values where applicable.

Failing Silently When "Falsey" Arguments Are Passed

Fail early, and fail noisily. You don't want to chase production bugs caused because someone forgot to pass an argument and have no indication that the argument was required. If they truly are required, throw errors:

var addMod = function(modContent, sectionName, sessionName) {
    if (!modContent) {
        throw new Error("Missing required argument: modContent");
    }

    if (!sessionName) {
        throw new Error("Missing required argument: sessionName");
    }

    if (!sessionName) {
        throw new Error("Missing required argument: sessionName");
    }

It makes for extra code, but development and maintenance become easier because you get clear errors (and stack traces). This really is a judgement call. There is a fine line between bullet proofing your code and expecting some level of intelligence from the people using it, especially for dynamically typed languages like JavaScript. If you have a public API, document it thoroughly. And "public" could mean "pushed to GitHub" or "used by coworkers at work."

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your helpful reply. I'll get to fixing everything ASAP \$\endgroup\$ – user21697 Nov 26 '14 at 20:17

Your Answer

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