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 could I 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).
<div id="frameHome">
<div align="center">
<p><strong>Math Quiz App</strong></p>
<p><input type="button" id="buttonPlay" value="Play"> <input type="button" id="buttonSettings" value="Settings"> <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="<"> <span id="divQues">10</span> <input type="button" id="buttonQuesPlus" value=">">
</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="<"> <span id="divTime">2:00</span> <input type="button" id="buttonTimePlus" value=">">
</p>
<p>
<input type="button" id="buttonCancel" value="Cancel"> <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>
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.