Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm working on my first web app and would like some feedback on the code I have completed. I asked for a similar code review a few weeks ago but I have since improved my structure and added some user adjustable settings.

The app is a simple math quiz (addition, subtraction, multiplication, & division questions) where users can adjust several settings based on their needs. I did my best to make it object-oriented and am curious how I could improve it?

I purchased a few HTML5 books from Amazon and read through them but like most resources (including Mozilla Developer Network) they did a great job of explaining everything but did not really show any examples of how to structure a complete web app with settings, etc.

My approach to structuring was to create a div for each "frame". A "frame" for the title screen, about screen, settings screen, quiz screen, and results screen. Only one "frame" is ever shown at a time and each "frame" has its own code area. Is this a good structure or could it be improved? Could I improve the way I load the current settings and update the settings when they're saved?

The code below includes the complete app structure, an about screen, and a settings screen that can be adjusted and saved by the user (for the session only, adding storage later).

Thanks in advance for your feedback.

Fiddle: http://jsfiddle.net/Xfyju/2/

<div id="frameHome">
<div align="center">
<p><strong>Math Quiz App</strong></p>
<p><input type="button" id="buttonPlay" value="Play"> &nbsp;&nbsp;&nbsp; <input type="button" id="buttonSettings" value="Settings"> &nbsp;&nbsp;&nbsp; <input type="button" id="buttonAbout" value="About"></p>
</div>
</div>

<div id="frameAbout">
<div align="center">
<p><strong>About</strong></p>
<p>Math Quiz App<br />
Version 1.0</p>
<p><input type="button" id="buttonOK" value="OK"></p>
</div>
</div>

<div id="frameSettings">
<div align="center">
<p><strong>Settings</strong></p>
<p>Operators: 
<input type="checkbox" name="checkboxOperators" id="add" /> 
Addition
<input type="checkbox" name="checkboxOperators" id="sub" /> 
Subtraction
<input type="checkbox" name="checkboxOperators" id="mul" /> 
Multiplication
<input type="checkbox" name="checkboxOperators" id="div" /> 
Division
</p>
<p>Number of questions: 
<input type="button" id="buttonQuesMinus" value="&lt;"> <span id="divQues">10</span> <input type="button" id="buttonQuesPlus" value="&gt;">
</p>
<p>Answer style:
<input type="radio" name="radioAnswerStyle" id="fillBlank" />
Fill in the Blank
<input type="radio" name="radioAnswerStyle" id="multipleChoice" /> 
Multiple Choice
</p>
<p>Timer:
<input type="radio" name="radioTimer" id="off" />
Off
<input type="radio" name="radioTimer" id="countdown" /> 
Countdown
<input type="radio" name="radioTimer"  id="elapsed" />
Elapsed
</p>
<p>Time: 
<input type="button" id="buttonTimeMinus" value="&lt;"> <span id="divTime">2:00</span> <input type="button" id="buttonTimePlus" value="&gt;">
</p>
<p>
<input type="button" id="buttonCancel" value="Cancel"> &nbsp;&nbsp;&nbsp; <input type="button" id="buttonSave" value="Save">
</p>
</div>
</div>

<div id="frameQuiz">
Quiz
</div>

<div id="frameResults">
Results
</div>

<script>
// Variables
// Settings
var operators = ["add", "sub", "mul", "div"]; // add, sub, mul, AND div
var totalQuestions = 10; // 5 TO 100
var timerStyle = "countdown"; // off OR countdown OR elapsed
var countdownTime = 120; // 30 TO 3600
var answerStyle = "multipleChoice"; // multipleChoice OR fillBlank

// Frames
var frameHome = document.getElementById("frameHome");
var frameAbout = document.getElementById("frameAbout");
var frameSettings = document.getElementById("frameSettings");
var frameQuiz = document.getElementById("frameQuiz");
var frameResults = document.getElementById("frameResults");

var frames = [frameHome, frameAbout, frameSettings, frameQuiz, frameResults];

// Displays frame and hides all other frames
function gotoFrame(frameID) {
    for (var i = 0; i < frames.length; i++) {
        if (frameID == frames[i].id) {
            // Show frame
            frames[i].style.display = "block";
        } else {
            // Hide frame
            frames[i].style.display = "none";
        }
    }

    frameCode(frameID);
}

// Code for each frame, code runs when frame is displayed
function frameCode(frameID) {
    switch (frameID) {


        case "frameHome":
            buttonPlay.onclick = function () { gotoFrame("frameQuiz"); };
            buttonSettings.onclick = function () { gotoFrame("frameSettings"); };
            buttonAbout.onclick = function () { gotoFrame("frameAbout"); };
            break;


        case "frameAbout":
            buttonOK.onclick = function () { gotoFrame("frameHome"); };
            break;


        case "frameSettings":
            // Operators  ----------
                // Get element
                var checkboxOperators = document.getElementsByName("checkboxOperators");

                // Uncheck all
                for (var i = 0; i < checkboxOperators.length; i++) {
                    document.getElementById(checkboxOperators[i].id).checked = false;
                }

                // Check operators based on operators array
                for (var i = 0; i < operators.length; i++) {
                    document.getElementById(operators[i]).checked = true;
                }
            // ------------------------------

            // Number of questions ----------
                var tempTotalQuestions = totalQuestions;

                divQues.innerHTML = tempTotalQuestions;

                // Questions minus
                buttonQuesMinus.onclick = function() { 
                    if (tempTotalQuestions > 5) {
                        tempTotalQuestions -= 5;
                    }

                    divQues.innerHTML = tempTotalQuestions;
                };
                // Questions plus
                buttonQuesPlus.onclick = function() { 
                    if (tempTotalQuestions < 100) {
                        tempTotalQuestions += 5;
                    }

                    divQues.innerHTML = tempTotalQuestions;
                };
            // ------------------------------

            // Answer style ----------
                document.getElementById(answerStyle).checked = true;
            // ------------------------------

            // Timer style ----------
                document.getElementById(timerStyle).checked = true;
            // ------------------------------

            // Countdown time ----------
                var tempCountdownTime = countdownTime;

                divTime.innerHTML = formatTime(tempCountdownTime);

                // Time minus
                buttonTimeMinus.onclick = function() { 
                    if (tempCountdownTime > 30) {
                        tempCountdownTime -= 30;
                    }

                    divTime.innerHTML = formatTime(tempCountdownTime);
                };
                // Time plus
                buttonTimePlus.onclick = function() { 
                    if (tempCountdownTime < 3600) {
                        tempCountdownTime += 30;
                    }

                    divTime.innerHTML = formatTime(tempCountdownTime);
                };
            // ------------------------------

            // Cancel button ----------
                buttonCancel.onclick = function () { gotoFrame("frameHome"); };
            // ------------------------------

            // Save button ----------
                buttonSave.onclick = function () {
                    // Save updated settings

                    // Operators
                    // Empty array
                    operators.length = 0;

                    for (var i = 0; i < checkboxOperators.length; i++) {
                        if (document.getElementById(checkboxOperators[i].id).checked == true) {
                            operators.push(checkboxOperators[i].id);    
                        }
                    }

                    // Number of questions
                    totalQuestions = tempTotalQuestions;

                    // Answer style
                    var radioAnswerStyle = document.getElementsByName("radioAnswerStyle");

                    for (var i = 0; i < radioAnswerStyle.length; i++) {                           
                        if (radioAnswerStyle[i].checked) {
                            answerStyle = radioAnswerStyle[i].id;
                        }
                    }

                    // Timer style
                    var radioTimer = document.getElementsByName("radioTimer");

                    for (var i = 0; i < radioTimer.length; i++) {                         
                        if (radioTimer[i].checked) {
                            timerStyle = radioTimer[i].id;
                        }
                    }

                    // Countdown time
                    countdownTime = tempCountdownTime;

                    gotoFrame("frameHome");
                };
            // ------------------------------
            break;


        case "frameQuiz":

            break;


        case "frameResults":

            break;


    }
}

// Format time
// Formats time as minutes and seconds (0:00)
function formatTime(time) {
    var minutes = Math.floor(time / 60);
    var seconds = time - (minutes * 60);

    // Add 0 in front of seconds less than 10
    if (seconds < 10) {
        seconds = "0" + seconds;
    }

    return minutes + ":" + seconds;
}

// Go to home frame when app opens
gotoFrame("frameHome");
</script>
share|improve this question

1 Answer 1

If you introduced a client-side MVVM framework, I think you would get the modularization that you are looking for. I have worked with, and really enjoy, knockout. Each frame could have it's own viewmodel, and you could create a single viewmodel that controls the flow logic between them.

For example, your settings frame would be built off of a SettingsViewModel that would hold all of the JavaScript, and perhaps contain a method on it that would return currentSettings to be used in other frames. Additionally, you can test the behavior of this view model independent of the representation of it (the HTML). Overall, this leads to a cleaner separation of concerns (application logic and presentation).

Here's a fiddle of a subset of the settings frame.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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