2
\$\begingroup\$

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).

Fiddle

<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>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 3, 2013 at 7:24
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

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.

answered Feb 3, 2013 at 22:20
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.