2
\$\begingroup\$

Maybe this is fine the way it is, but it seems messy. Basically I have dynamically generated HTML that will display a list of songs in a given folder. Each song has an ID, and I want to add a button that allows you to add every song in the folder.

Right now it's creating the add all button with the array of song IDs as an argument. It just feels messy since the list can be incredibly long and is now exposed to the HTML, but maybe it's fine?

It's currently working like this:

if (songidarray.length > 0)
    backbuttonhtml += "<a href=\"javascript:applib.addentirefolder('"+songidarray+"');\" type=\"button\">Add Entire Folder</a>";
window.$("#folderpagebackbutton").html(backbuttonhtml).trigger('create');

I could make the songidarray a global variable and make the button static. But that seems like a cop out for some reason, maybe I'm just asking a stupid question and that's the answer right there. Wondering if there is a way to do it without using a global variable, or if there's nothing wrong with using one.

\$\endgroup\$
  • 1
    \$\begingroup\$ I would not make a variable global. I am not entirely sure what you are concerned about though. Could you perhaps add some more code for some more context? \$\endgroup\$ – Simon Forsberg Feb 27 '15 at 10:42
1
\$\begingroup\$

Instead of making the songidarray global, use a closure.

$(window).load(function () { // for example, or wherever you get the data
    var songidarray = [1,2,3,4,5,9];
    // var songidarray = [];

    var $button = $("<button id='addEntireFolderButton' type='button'>")
        .html("Add Entire Folder")
        .prop("disabled", songidarray.length == 0)
        .click(function () {
            applib.addentirefolder(songidarray);
        });
    $('#folderpagebackbutton').append($button);
});

The click callback above will maintain a live reference to songidarray and will pass it to the addentirefolder function when triggered. The contents of the array stay out of the HTML.

And since this is Code Review:

The type attribute on <a> elements is a hint for the MIME type of the referenced resource. Use the class attribute if you need to tag an element for styling. On a <button> element, type="button" makes it do nothing by default as opposed to submitting or resetting any containing form.

\$\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.