3
\$\begingroup\$

I'd rather not use this long code. I want to retrieve table/rows from database and insert into JavaScript to create a table in HTML.

This is what it looks like so far and I'd like to keep the design if possible:

https://i.sstatic.net/rDUqk.jpg 

var time = 10
var wrongs = 0;
var wrongs2 = 0;
var next = 0;
var right = 0;
var right2 = 0;
function timer(){ 
	if(!time<1){
	time = time - 1
	refresh()
	}else{
		randquestion()
	wrongs = wrongs + 1
	wrongs2 = wrongs2 + 1
	time = time + 10
	result.innerHTML = "<b>Failed!</b>"
	wrong.innerHTML = "<b>Wrong | "+wrongs+"</b>"
	}
}
function start(){
	if (next==0){
		next = 1;
	}
if (next==1){	
	refresh()
	enable()
	randquestion()
	update = setInterval("timer()",1000)
	next = 2;
	}
}
function stop(){
	next = 0;
if (next == 0){	
	randquestion()
	disable()
	time = 10
	wrongs = 0
	right = 0
	refresh()
	window.clearInterval(update);
	}
}
function changepw(){
	window.location.href = 'newpw.php';
}
function logout(){
	window.location.href = 'logout.php?logout';
}
var sel
var questions
var namev
var answers
var eng
function onenter(e){
	if (e.keyCode == 13) {
	questions = document.getElementById("questionsbox").innerHTML;
	answers = document.getElementById("answer").value;
	time = 10
	refresh()
	if (answers==namev){
		right = right + 1
		right2 = right2 + 1
		test.innerHTML = "<b>Correct Answer | "+namev+"</b>"
		randquestion()
		}else{
			wrongs = wrongs + 1
			wrongs2 = wrongs2 + 1
			test.innerHTML = "<b>Correct Answer | "+namev+"</b>"
			randquestion()
		}
	}
}
function findquestion(){
	if (questions=="日本語"){
		te="japan"
	}
}
function disable(){
	document.getElementById('answer').disabled = true;
}
function enable(){
	document.getElementById('answer').disabled = false;
}
function randquestion(){
	sel = Math.floor(Math.random() * 17) + 1 
	document.getElementById("answer").value = '';
	if (sel==1){
		namev = "nihongo" 
		questionsbox.innerHTML = "<b>日本語</b>"
	}
	if (sel==2){
		namev = "kyou" 
		questionsbox.innerHTML = "<b>今日</b>"
	}
	if (sel==3){
		namev = "iku" 
		questionsbox.innerHTML = "<b>行く</b>"
	}
	if (sel==4){
		namev = "ohayou" 
		questionsbox.innerHTML = "<b>おはよう</b>"
	}
	if (sel==5){
		namev = "asa" 
		questionsbox.innerHTML = "<b>朝</b>"
	}
	if (sel==6){
		namev = "nihon" 
		questionsbox.innerHTML = "<b>日本</b>"
	}
	if (sel==7){
		namev = "anata" 
		questionsbox.innerHTML = "<b>貴方</b>"
	}
	if (sel==8){
		namev = "kore" 
		questionsbox.innerHTML = "<b>之</b>"
	}
	if (sel==9){
		namev = "sore" 
		questionsbox.innerHTML = "<b>其</b>"
	}
	if (sel==10){
		namev = "are" 
		questionsbox.innerHTML = "<b>荒</b>"
	}
	if (sel==11){
		namev = "shigoto" 
		questionsbox.innerHTML = "<b>仕事</b>"
	}
	if (sel==12){
		namev = "nomu" 
		questionsbox.innerHTML = "<b>飲む</b>"
	}
	if (sel==13){
		namev = "naru" 
		questionsbox.innerHTML = "<b>成る</b>"
	}
	if (sel==14){
		namev = "sekai" 
		questionsbox.innerHTML = "<b>世界</b>"
	}
	if (sel==15){
		namev = "mizu" 
		questionsbox.innerHTML = "<b>水</b>"
	}
	if (sel==16){
		namev = "kenkyou" 
		questionsbox.innerHTML = "<b>研究</b>"
	}
	if (sel==17){
		namev = "sakura" 
		questionsbox.innerHTML = "<b>桜</b>"
	}
	if (sel==18){
		namev = "sensei" 
		questionsbox.innerHTML = "<b>先生</b>"
	}
	if (sel==19){
		namev = "kawaii" 
		questionsbox.innerHTML = "<b>かわいい</b>"
	}
	if (sel==20){
		namev = "yatta" 
		questionsbox.innerHTML = "<b>やった</b>"
	}
	if (sel==21){
		namev = "" 
		questionsbox.innerHTML = "<b></b>"
	}
	if (sel==22){
		namev = "" 
		questionsbox.innerHTML = "<b></b>"
	}
}
function refresh(){
	result.innerHTML = "<b>Timer | "+time+"</b>"
	wrong.innerHTML = "<b>Wrong | "+wrongs+"</b>"
	wrong2.innerHTML = "<b>Total Wrong | "+wrongs2+"</b>"
	correct.innerHTML = "<b>Correct | "+right+"</b>"
	correct2.innerHTML = "<b>Total Correct | "+right2+"</b>"
}
function table(){
		r1c1.innerHTML = "<b>nihongo</b>"
		r1c2.innerHTML = "<b>日本語</b>"
		r1c3.innerHTML = "<b>japanese</b>"
		
		r2c1.innerHTML = "<b>kyou</b>"
		r2c2.innerHTML = "<b>今日</b>"
		r2c3.innerHTML = "<b>today</b>"
		
		r3c1.innerHTML = "<b>iku</b>"
		r3c2.innerHTML = "<b>行く</b>"
		r3c3.innerHTML = "<b>go</b>"
		
		r4c1.innerHTML = "<b>ohayou</b>"
		r4c2.innerHTML = "<b>おはよう</b>"
		r4c3.innerHTML = "<b>good morning</b>"
		
		r5c1.innerHTML = "<b>asa</b>"
		r5c2.innerHTML = "<b>朝</b>"
		r5c3.innerHTML = "<b>morning</b>"
		
		r6c1.innerHTML = "<b>nihon</b>"
		r6c2.innerHTML = "<b>日本</b>"
		r6c3.innerHTML = "<b>japan</b>"
		
		r7c1.innerHTML = "<b>anata</b>"
		r7c2.innerHTML = "<b>貴方</b>"
		r7c3.innerHTML = "<b>you</b>"
		
		r8c1.innerHTML = "<b>kore</b>"
		r8c2.innerHTML = "<b>之</b>"
		r8c3.innerHTML = "<b>this</b>"
		
		r9c1.innerHTML = "<b>sore</b>"
		r9c2.innerHTML = "<b>其</b>"
		r9c3.innerHTML = "<b>there</b>"
		
		r10c1.innerHTML = "<b>are</b>"
		r10c2.innerHTML = "<b>荒</b>"
		r10c3.innerHTML = "<b>that</b>"
		
		r11c1.innerHTML = "<b>shigoto</b>"
		r11c2.innerHTML = "<b>仕事</b>"
		r11c3.innerHTML = "<b>work</b>"
		
		r12c1.innerHTML = "<b>nomu</b>"
		r12c2.innerHTML = "<b>飲む</b>"
		r12c3.innerHTML = "<b>to drink</b>"
		
		r13c1.innerHTML = "<b>naru</b>"
		r13c2.innerHTML = "<b>成る</b>"
		r13c3.innerHTML = "<b>to become</b>"
		
		r14c1.innerHTML = "<b>sekai</b>"
		r14c2.innerHTML = "<b>世界</b>"
		r14c3.innerHTML = "<b>world</b>"
		
		r15c1.innerHTML = "<b>mizu</b>"
		r15c2.innerHTML = "<b>水</b>"
		r15c3.innerHTML = "<b>water</b>"
		
		r16c1.innerHTML = "<b>kenkyou</b>"
		r16c2.innerHTML = "<b>研究</b>"
		r16c3.innerHTML = "<b>study</b>"
		
		r17c1.innerHTML = "<b>sakura</b>"
		r17c2.innerHTML = "<b>桜</b>"
		r17c3.innerHTML = "<b>cherry blossom</b>"
		
		r18c1.innerHTML = "<b>sensei</b>"
		r18c2.innerHTML = "<b>先生</b>"
		r18c3.innerHTML = "<b>teacher</b>"
		
		r19c1.innerHTML = "<b>kawaii</b>"
		r19c2.innerHTML = "<b>かわいい</b>"
		r19c3.innerHTML = "<b>cute</b>"
		
		r20c1.innerHTML = "<b>yatta</b>"
		r20c2.innerHTML = "<b>やった</b>"
		r20c3.innerHTML = "<b>hooray</b>"
		
		r21c1.innerHTML = "<b>asa</b>"
		r21c2.innerHTML = "<b>朝</b>"
		r21c3.innerHTML = "<b>morning</b>"
		
		r22c1.innerHTML = "<b>asa</b>"
		r22c2.innerHTML = "<b>朝</b>"
		r22c3.innerHTML = "<b>morning</b>"
		
		r23c1.innerHTML = "<b>asa</b>"
		r23c2.innerHTML = "<b>朝</b>"
		r23c3.innerHTML = "<b>morning</b>"
		
		r24c1.innerHTML = "<b>asa</b>"
		r24c2.innerHTML = "<b>朝</b>"
		r24c3.innerHTML = "<b>morning</b>"
		
		r25c1.innerHTML = "<b>asa</b>"
		r25c2.innerHTML = "<b>朝</b>"
		r25c3.innerHTML = "<b>morning</b>"
		
		r26c1.innerHTML = "<b>asa</b>"
		r26c2.innerHTML = "<b>朝</b>"
		r26c3.innerHTML = "<b>morning</b>"
		
		rc1.innerHTML = "<b>asa</b>"
		rc2.innerHTML = "<b>朝</b>"
		rc3.innerHTML = "<b>morning</b>"
		rc3.innerHTML = "<b>morning</b>"
		
		//rc1.innerHTML = "<b>asa</b>"
		//rc2.innerHTML = "<b>朝</b>"
		//rc3.innerHTML = "<b>morning</b>"
}
<?php
session_start();
include_once 'dbconnect.php';
if(!isset($_SESSION['user']))
{
 header("Location: index.php");
}
echo $_GET['right'];
?>
<!DOCTYPE html>
<html>
	<header>
		<title>Quiz</title>
		<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
		<script type="text/javascript" src="js/reload.js"></script>
		<link href="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
		<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>
		<script src="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>
		
		<nav>
			<ul class="nav nav-pills">
				<li><a href="home.php">Home</a></li>
				<li><a href="quiz.php">Quiz</a></li>
				<li role="presentation" class="active"><a href="">Study</a></li>
				<li><a href="https://www.facebook.com/groups/1606566483004459/"target="_blank">Facebook</a></li>
			</ul>
		</nav>
	</header>
	
	
	<body onload="table()">
		<div class="col-sm-2 col-xs-2">
					<div class="panel panel-default">
					<div onmousedown='return false;'class="panel-heading"><b><?php echo "Welcome ".$_SESSION['user']. ".<br>";?></b></div>
					<div class="panel-body">
					<div>
							<input onclick="logout()" class="l btn btn-default2" type="submit" value="Logout" />
							<input onclick="changepw()" class="l btn btn-default2" type="submit" value="Change Password" />
				</div>
			</div>
		</div>
	</div>
	
 <div class="container">
 <table class="table table-default">
 <thead>
 <tr>
 <th> <div class="list-group-item"> Romanji</th> </div>
 <th> <div class="list-group-item"> ひらがな|カタカナ|漢字</th> </div>
 <th> <div class="list-group-item"> English</th> </div>
 </tr>
 </thead>
 <tbody>
 <tr>
 <td> <div id="r1c1" class="list-group-item"></td> </div>
 <td> <div id="r1c2" class="list-group-item"></td> </div>
 <td> <div id="r1c3" class="list-group-item"></td> </div>
 </tr>
 <tr>
		<td> <div id="r2c1" class="list-group-item"></td> </div>
 <td> <div id="r2c2" class="list-group-item"></td> </div>
 <td> <div id="r2c3" class="list-group-item"></td> </div>
 </tr>
 <tr>
 <td> <div id="r3c1" class="list-group-item"></td> </div>
 <td> <div id="r3c2" class="list-group-item"></td> </div>
 <td> <div id="r3c3" class="list-group-item"></td> </div>
 </tr>
	 <tr>
 <td> <div id="r4c1" class="list-group-item"></td> </div>
 <td> <div id="r4c2" class="list-group-item"></td> </div>
 <td> <div id="r4c3" class="list-group-item"></td> </div>
 </tr>
	 <tr>
 <td> <div id="r5c1" class="list-group-item"></td> </div>
 <td> <div id="r5c2" class="list-group-item"></td> </div>
 <td> <div id="r5c3" class="list-group-item"></td> </div>
 </tr>
	 <tr>
 <td> <div id="r6c1" class="list-group-item"></td> </div>
 <td> <div id="r6c2" class="list-group-item"></td> </div>
 <td> <div id="r6c3" class="list-group-item"></td> </div>
 </tr><tr>
 <td> <div id="r7c1" class="list-group-item"></td> </div>
 <td> <div id="r7c2" class="list-group-item"></td> </div>
 <td> <div id="r7c3" class="list-group-item"></td> </div>
 </tr><tr>
 <td> <div id="r8c1" class="list-group-item"></td> </div>
 <td> <div id="r8c2" class="list-group-item"></td> </div>
 <td> <div id="r8c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r9c1" class="list-group-item"></td> </div>
 <td> <div id="r9c2" class="list-group-item"></td> </div>
 <td> <div id="r9c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r10c1" class="list-group-item"></td> </div>
 <td> <div id="r10c2" class="list-group-item"></td> </div>
 <td> <div id="r10c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r11c1" class="list-group-item"></td> </div>
 <td> <div id="r11c2" class="list-group-item"></td> </div>
 <td> <div id="r11c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r12c1" class="list-group-item"></td> </div>
 <td> <div id="r12c2" class="list-group-item"></td> </div>
 <td> <div id="r12c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r13c1" class="list-group-item"></td> </div>
 <td> <div id="r13c2" class="list-group-item"></td> </div>
 <td> <div id="r13c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r14c1" class="list-group-item"></td> </div>
 <td> <div id="r14c2" class="list-group-item"></td> </div>
 <td> <div id="r14c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r15c1" class="list-group-item"></td> </div>
 <td> <div id="r15c2" class="list-group-item"></td> </div>
 <td> <div id="r15c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r16c1" class="list-group-item"></td> </div>
 <td> <div id="r16c2" class="list-group-item"></td> </div>
 <td> <div id="r16c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r17c1" class="list-group-item"></td> </div>
 <td> <div id="r17c2" class="list-group-item"></td> </div>
 <td> <div id="r17c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r18c1" class="list-group-item"></td> </div>
 <td> <div id="r18c2" class="list-group-item"></td> </div>
 <td> <div id="r18c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r19c1" class="list-group-item"></td> </div>
 <td> <div id="r19c2" class="list-group-item"></td> </div>
 <td> <div id="r19c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="r20c1" class="list-group-item"></td> </div>
 <td> <div id="r20c2" class="list-group-item"></td> </div>
 <td> <div id="r20c3" class="list-group-item"></td> </div>
 </tr>
	 </tr><tr>
 <td> <div id="rc1" class="list-group-item"></td> </div>
 <td> <div id="rc2" class="list-group-item"></td> </div>
 <td> <div id="rc3" class="list-group-item"></td> </div>
 </tr>
 </tbody>
 </table>
</div>
	
	</body>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js" integrity="sha384-0mSbJDEHialfmuBBQP6A4Qrprq5OVfW37PRR3j5ELqxss1yVqOtnepnHVP9aJ7xS" crossorigin="anonymous"></script>
</html>

Sᴀᴍ Onᴇᴌᴀ ♦
29.5k16 gold badges45 silver badges201 bronze badges
asked Aug 29, 2016 at 6:53
\$\endgroup\$
3
  • \$\begingroup\$ Use else if in randquestion() or better, use an array of objects where each object store namev and the innerHTML. \$\endgroup\$ Commented Aug 29, 2016 at 7:29
  • \$\begingroup\$ @Tushar A switch is definitely the way to go. \$\endgroup\$ Commented Aug 29, 2016 at 8:02
  • \$\begingroup\$ Invalid order of tags in the table, should be <td><div...></div></td>. \$\endgroup\$ Commented Aug 29, 2016 at 17:49

3 Answers 3

3
\$\begingroup\$
  1. Global Variables: All the variables defined are global, which is bad practice. Avoid declaring global variables. Wrap the code in IIFE and define variables using var keyword.
  2. Naming Conventions: The names of the variable should give indication of it's purpose. The names should be in camelCase format.
  3. Caching: Cache the DOM element reference and use it. This will save the time to dive into DOM again.
  4. Missing Semicolons: Although, this will not cause any errors since JavaScript's Automatic Semicolon Insertion(ASI) will add the semicolons, it's good practice to add them. Also, depending on ASI can cause the semicolon to add someplace where the symantic will be changes. Ex: When declaring multiple variables if we forgot a comma, the successive variables will be declared as Global.
  5. Comparison: To compare variables use strict comparison operators(=== and !==). Loose comparison will cast the operands before comparing variables which could lead into unexpected behavior.
  6. Using else if ladder: For mutually exclusive conditions, use else if. If the condition is satisfied, the next conditions in else if are not executed. Or switch statement can also be used.
  7. Nested for: Use nested for loops to iterate over the <table> and update the values of each cell.(table())
  8. Appending Markup: Instead of appending markup(like <b> element in refresh()), IF POSSIBLE, add the markup in the HTML and only update the text-content of the element through JavaScript. This will avoid adding multiple nested elements if the function is repeatedly called.
  9. Comparing Variables: !time < 1 is equivalent to time >= 1 which is more readable and easy to understand. Also, this will save an operation(Logical Not).
  10. Using string parameter to setInterval: Don't use string parameter to the setInterval and setTimeout functions. You can pass the function reference as setInterval(timer, 1000)
  11. Un-Needed Code: In stop() first next variable is assigned to zero and then it is compared with zero which is always going to evaluate as true. The if condition is not necessary there.
  12. Similar Functions: The functions changepw() and logout() are similar. Both are redirecting to some page. This can be combined into one function redirect(url) accepting a parameter as redirect URL and redirect to that URL. Same is with disable() and enable() functions.
  13. Logical Error & Dead Code: The expression Math.floor(Math.random() * 17) + 1 will always produce value between one and seventeen. So, the code written for comparing it 18 through 22 is dead code, which can be removed safely. Or, if you want to generate the random number between one to twenty-two replace the 17 by 22 in that statement.
  14. Binding Events: Use addEventListener to bind events on elements. As in the current code, no event binding code is shown, if inline binding in HTML is used, use addEventListener to bind events.
answered Aug 29, 2016 at 9:12
\$\endgroup\$
1
\$\begingroup\$

Use a Switch.

The moment you start doing things like the following, you got yourself a maintainability problem:

if (sel==4){
 namev = "ohayou" 
 questionsbox.innerHTML = "<b>おはよう</b>"
}
if (sel==5){
 namev = "asa" 
 questionsbox.innerHTML = "<b>朝</b>"
}
if (sel==6){
 namev = "nihon" 
 questionsbox.innerHTML = "<b>日本</b>"
}
if (sel==7){
 namev = "anata" 
 questionsbox.innerHTML = "<b>貴方</b>"
}

What if you decide to rename sel? You'd have to rename it everywhere!

switch (sel) {
 case 4:
 namev = "ohayou" 
 questionsbox.innerHTML = "<b>おはよう</b>"
 break;
 case 5:
 namev = "asa" 
 questionsbox.innerHTML = "<b>朝</b>"
 break;
 case 6:
 namev = "nihon" 
 questionsbox.innerHTML = "<b>日本</b>"
 break;
 case 7:
 namev = "anata" 
 questionsbox.innerHTML = "<b>貴方</b>"
 break;
}

Much neater! Easier to maintain, you're not needlessly repeating yourself (those break are more like parentheses in this regard) and it's immediately clear all those conditionals rely on the same variable. The ease of understanding your code just went up a couple of points.

answered Aug 29, 2016 at 8:02
\$\endgroup\$
2
  • \$\begingroup\$ I agree that switch will make code a bit better but there is something wrong with design if you have to put statements that long in your logic. Putting all those words in separate files would make code more readable. \$\endgroup\$ Commented Aug 29, 2016 at 14:01
  • 1
    \$\begingroup\$ This could use a an associative array in PHP or an array of strings that are sorted properly as well. \$\endgroup\$ Commented Aug 29, 2016 at 16:25
1
\$\begingroup\$

JavaScript

  • Cache DOM references - anytime document.getElementById() is called multiple times, it is better to store that reference in a variable (or const if you are supporting es-6)

  • As other's have suggested, a switch statement can simplify the if-else blocks of randquestion(). Another technique is to use a mapping of values. For instance, ones could define an array for the names and questions:

    var namevs = ["nihongo", "kyou", "iku", ..., "yatta"];
var questions = ["日本語", "今日", "行く", ..., "やった<"];

    Then use those values by index, as in the sample below. Notice that the value assigned to sel does not need to be increased after taking the floor of the random number between 0 and 17, since array index is 0-based.

    sel = Math.floor(Math.random() * 17);
namev = namevs[sel];
questionsbox.innerHTML = "<b>" + questions[sel] + "</b>";

  • The random value in sel will at most have a value of 17. Thus the last 6 conditional code blocks of randquestion() will never get executed. If those last few questions need to be accessible, then update the math for generating sel - if the array mappings above are used, one could utilize those:

    sel = Math.floor(Math.random() * namevs.length);

  • There are lines of code in the table() function that reference DOM elements implicitly by id attribute, which do not exist in the DOM:

    r21c1.innerHTML = "<b>asa</b>"

    This leads to an error in the browser console:

    Uncaught ReferenceError: r21c1 is not defined

HTML

  • In the snippet HTML, jQuery version 1.12.4 is linked in the <head> section, then version 1.11.3 is included towards the end. It doesn't appear that either are used in your JavaScript code, just used for Bootstrap.

  • The table headers and cells appear to have <div> tags that are not closed before the cell is closed. For example:

     <th> <div class="list-group-item"> Romanji</th> </div>

    Valid HTML would look like the following:

     <th> <div class="list-group-item"> Romanji</div> </th>

PHP

I see the following line:

echo $_GET['right'];

Which would come before the HTML content... is that just left in for debugging purposes? It would be advisable not to echo content before the start of the HTML content.

answered May 8, 2018 at 21:45
\$\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.