Karma Sutra game

Question

I have created a simple Javascript game and want some second opinions on the code. How can it be done more efficiently for example?

Please see the code below, plus a link to a live working version on github.

Code to github

Thanks in advance.

(function(){

// The intro element. This element displays some introductory text and a button to start 
// playing
var intro = document.getElementById("intro");

// hide the listcontainer at first
var listContainer = document.getElementById('positions');
listContainer.style.display = 'none';

var sectionTitle = document.getElementById("section-title");
sectionTitle.style.display = 'none';

// the button to start the game
var displayPositions = document.getElementById("playButton");

// when we click the display button, we hide the intro element then display the
// list of options and section titles
displayPositions.onclick = function() {
    intro.style.display = 'none';
    sectionTitle.style.display = 'block';
    listContainer.style.display = 'block';
};

/*
 * Create a function to display all positions
 */
function createPositions() {

    // the array of positions
    var positions = [
        "Sixty-Nine",
        "Missionary",
        "Doggy style",
        "Rodeo",
        "Reverse Cowgirl",
        "Girl on top",
        "Zeus",
        "Venus",
        "Workout"
    ];

    // get hold of the list container on the page
    var listContainer = document.getElementById('positions');

    // loop through all the positions and place each one in an li element 
    for(var i = 0, n = positions.length; i < n; i++) {

        // create an li element
        var listElement = document.createElement("button");

        // set the attribute and ID to be the position index
        var listValue = listElement.setAttribute("value", "position-" + [i]);
        listElement.setAttribute("id", "position-" + [i]);
        listElement.setAttribute("class", "choice-buttons");

        // create the text to be placed in the element
        var listContent = document.createTextNode(positions[i]);

        // append the text to the li 
        listElement.appendChild(listContent);

        // append the li to the ul 
        listContainer.appendChild(listElement);
    }
}

// call function 
createPositions();

/*
 * Create a function that gets the selections 
 * from the user
 */
function getSelection() {

    // create an empty array
    var selections = [];
    var femaleSelections = [];
    var maleSelections = [];

    // get the main container of the page
    var container = document.getElementById("main-container");

    // get this again to access the dom element
    var listContainer = document.querySelector('#positions');

    // get the element where we put the answer. This is where we will see the final results
    var answerElement = document.getElementById("chosenPositions");
    var answerP = document.createElement("P");

    // create a p element that we will display the final answer / chosen positions in
    answerElement.appendChild(answerP);

    // initially don't display this element
    answerElement.style.display = 'none';

    // start out with a default class
    container.className = "female";

    // store the choices as an array
    var choices = listContainer.querySelectorAll('button');

    // get the length of the choices so we can loop over all of them                
    var choicesLength = choices.length;

    // start loop 
    for(var i = 0; i < choicesLength; i++) {

        // for each element in the array, add an event listener to the
        // button which is clicked
        choices[i].addEventListener("click", function(){

            // the position, as in "sexual" position, is the value of the 
            // current element / button
            var position = this.getAttribute("value");

            // push that position into the selections array
            // selections.push(position);

            // for debugging - TODO: delete this:
            // console.log(selections);


            // First get the female selections
            if(femaleSelections.length < 2) {

                // push them into the femaleSections array
                femaleSelections.push(position);
                this.disabled = true;

                // DEBUG: 
                console.log(femaleSelections);

                // change the classname to male once female selections reach 2
                if(femaleSelections.length == 2) {
                    container.className = "male";
                    sectionTitle.innerHTML = 'Gents';
                    sectionTitle.classList.remove('slideInLeft');
                    sectionTitle.className += ' slideInRight';
                    selections.push(femaleSelections);
                }
            }

            // now get male selections
            else if(maleSelections.length < 2) {

                // push them into the maleSelections array
                maleSelections.push(position);
                this.disabled = true;

                // DEBUG: 
                console.log(maleSelections);

                // change the classname to results once the male sections reach 2
                // and disable all the buttons so no more selections can be made
                if(maleSelections.length == 2) {
                    container.className = "results";
                    sectionTitle.innerHTML = 'Results';
                    sectionTitle.classList.remove('slideInRight');
                    sectionTitle.className += ' tada';
                    selections.push(maleSelections);

                    // randomly get a selection from each array                 
                    var randomSelectionF = Math.floor(Math.random() * femaleSelections.length);
                    var randomSelectionM = Math.floor(Math.random() * maleSelections.length);

                    // get the male and female positions as text
                    var femalePosition = document.getElementById(femaleSelections[randomSelectionF]).innerHTML;
                    var malePosition = document.getElementById(maleSelections[randomSelectionM]).innerHTML;

                    // get the actually button we clicked on so we can add classes to it
                    var femaleSelected = document.getElementById(femaleSelections[randomSelectionF]);
                    var maleSelected = document.getElementById(maleSelections[randomSelectionM]);

                    femaleSelected.className += ' selected animated infinite pulse';
                    maleSelected.className += ' selected animated infinite pulse';

                    // if the two randmon choices are the same position, manually choose them.
                    // NOTE: I'm not sure if this is the best approach to this? 
                    if(femaleSelections[randomSelectionF] === maleSelections[randomSelectionM]) {
                        femalePosition = document.getElementById(femaleSelections[0]).innerHTML;
                        malePosition = document.getElementById(maleSelections[1]).innerHTML;
                    }

                    // Create the decision string, ie what your final positions will be     
                    var decision = femalePosition + " and " + malePosition;

                    // display the answer element                       
                    answerElement.style.display = 'block';

                    // Put the answer into the HTML element
                    answerP.appendChild(document.createTextNode("Looks like you'll be doing " + decision + " tonight!"));

                    // disable all the buttons so no more selections can be made
                    choices.forEach(function(element){
                        element.disabled = true;
                    })  ;

                    // get hold of the play again button
                    var playAgain = document.getElementById("startOver");

                    // to play again we reset all out variables to their
                    // original values 
                    playAgain.addEventListener("click", function(){
                        selections = [];
                        femaleSelections = [];
                        maleSelections = [];

                        container.className = "female";
                        sectionTitle.innerHTML = 'Ladies';
                        sectionTitle.classList.remove('tada');
                        sectionTitle.className += ' slideInLeft';
                        answerElement.style.display = 'none';
                        choices.forEach(function(element){
                            element.disabled = false;
                        })  ;
                        answerP.innerHTML = '';                 
                        femaleSelected.classList.remove('selected', 'animated', 'pulse', 'inifinte');
                        maleSelected.classList.remove('selected', 'animated', 'pulse', 'inifinte');
                    });

                }
            }

        });

    }       

}

// call the function
getSelection();

})();

Answer 1

Adding feedback in the form of hints like Select the first position, then Now select the second position would enhance UX.

Now let's nitpick.

• Split the huge multi-purpose function into smaller specific ones, group code by purpose
• Most of your comments are redundant and just add infonoise to self-explanatory code
• Use constants instead of magic values e.g. MAX_CHOICES instead of 2
• Instead of meaningless array in "position-" + [i] simply use "position-" + i

// Initialization
var intro = getByID('intro');
var container = getByID('main-container');
var choicesContainer = getByID('positions');
var sectionTitle = getByID('section-title');
var answer = getByID('chosenPositions');

// create positions
choicesContainer.insertAdjacentHTML('beforeend', [
        'Sixty-Nine', 'Missionary',      'Doggy style',
        'Rodeo',      'Reverse Cowgirl', 'Girl on top',
        'Zeus',       'Venus',           'Workout'
    ].map(function(position, i) {
        return '<button id="position-' + i + '" ' +
                       'class="choice-buttons">' + position +
               '</button>';
    }).join('')
);

var MAX_CHOICES = 2;
var choices = choicesContainer.querySelectorAll('button');
choices.ladies = [];
choices.gents = [];

getByID('playButton').onclick = play;
getByID('startOver').onclick = restart;
choices.forEach(function(c) { c.onclick = choose; });

// only intro is shown on start
hide(choicesContainer);
hide(sectionTitle);
hide(answer);

// ladies choose first
container.className = 'female';

• Use properties like .className directly instead of setAttribute when creating elements, the same applies to .value instead of getAttribute
• Use .textContent instead of creating a separate text node

// overall game control
function play() {
    hide(intro);
    show(sectionTitle);
    show(choicesContainer);
}

function restart() {
    container.className = 'female';
    sectionTitle.textContent = 'Ladies';
    sectionTitle.classList.remove('tada');
    sectionTitle.classList.add('slideInLeft');

    hide(answer);

    choices.ladies = [];
    choices.gents = [];
    deanimateChoice(ladiesChoice);
    deanimateChoice(gentsChoice);
    choices.forEach(function(c) { element.disabled = false; });
}

• Extract the huge click handler to a separate function instead of burying it inside a loop or move out the variables to group them with the other globally used ones.

// in-game control
function choose() {
    // prevent duplicate choices
    this.disabled = true;

    // Ladies first
    if (choices.ladies.length < MAX_CHOICES) {
        choices.ladies.push(this.id);

        // change the classname to male once female choices reach 2
        if (choices.ladies.length == MAX_CHOICES) {
            container.className = 'male';
            sectionTitle.textContent = 'Gents';
            sectionTitle.classList.remove('slideInLeft');
            sectionTitle.classList.add('slideInRight');
        }
    } else {
        choices.gents.push(this.id);

        if (choices.gents.length == MAX_CHOICES) {
            showAnswer();
        }
    }
}

• Return from function soon instead of having a huge else clause or rework it (extract)

// results
function showAnswer() {
    // change the classname to results once the male sections reach 2
    container.className = 'results';
    sectionTitle.textContent = 'Results';
    sectionTitle.classList.remove('slideInRight');
    sectionTitle.classList.add('tada');

    // and disable all the buttons so no more choices can be made
    choices.forEach(function(c) { c.disabled = true; });

    // randomly get a selection from each array                 
    do {
        var ladiesIndex = Math.floor(Math.random() * MAX_CHOICES);
        var gentsIndex = Math.floor(Math.random() * MAX_CHOICES);
    } while (ladiesIndex == gentsIndex);

    var ladiesChoice = getByID(choices.ladies[ladiesIndex]);
    var gentsChoice = getByID(choices.gents[gentsIndex]);
    animateChoice(ladiesChoice);
    animateChoice(gentsChoice);

    answer.textContent = "Looks like you'll be doing " + 
        ladiesChoice.textContent + ' and ' +
        gentsChoice.textContent +
        ' tonight!';
    show(answer);
}

• Add a function to get elements by id since you're using it a lot e.g. id('str') or $('str') and use it consistently (currently you have one querySelector).
• Add functions show(element) as well as hide that alter .style.display to up readability

// utilities
function getByID(ID) {
    return document.getElementById(ID);
}
function show(element) {
    element.style.display = 'block';
}
function hide(element) {
    element.style.display = 'none';
}
function animateChoice(element) {
    element.classList.add('selected', 'animated', 'infinite', 'pulse');
}
function deanimateChoice(element) {
    element.classList.remove('selected', 'animated', 'pulse', 'inifinte');
}

• Use classList methods consistently, don't mix with direct className modifications.
• Don't cache array length in for-loops if you don't iterate thousands or millions of times: nano-optimizations like these reduce readability
• Add event handlers consistently: either .onxxx or .addEventListener
• Pay attention to variable names: listContainer was redefined in nested scope.
• Don't needlessly declare variables in the top scope as it implies they would be used later. For example, displayPositions isn't needed, instead use id("playButton").onclick =
• Use single or double quotes consistently