JS Code Advice (greatly appreciated)

R

rupoopin

Hello,
I am not a good programmer. I wrote a javascript to check my lotto
numbers. I would like feedback on how I could make the script I wrote
'better'. Thanks in advance, I really look forward to seeing different
solutions to the script I wrote. I am sure there there is a much
better way to accomplish the end result.

Here is the code:

<html>
<head>
<title>Check Lotto Nums</title>
<script type="text/javascript">
var oSetOne=new Array("2","7","13","17","33","47");
var oSetOneIds=new Array("1","2","3","4","5","6");

var oSetTwo=new Array("12","19","22","31","32","41");
var oSetTwoIds=new Array("7","8","9","10","11","12");

var oSetThree=new Array("17","22","23","25","26","42");
var oSetThreeIds=new Array("13","14","15","16","17","18");

var oSetFour=new Array("14","21","28","29","37","46");
var oSetFourIds=new Array("19","20","21","22","23","24");

var oSetFive=new Array("1","5","6","15","18","35");
var oSetFiveIds=new Array("25","26","27","28","29","30");

var oSetSix=new Array("10","11","31","36","38","48");
var oSetSixIds=new Array("31","32","33","34","35","36");

var oSetSeven=new Array("3","4","15","23","36","49");
var oSetSevenIds=new Array("37","38","39","40","41","42");

var oSetEight=new Array("1","3","27","36","39","42");
var oSetEightIds=new Array("43","44","45","46","47","48");

var oSetNine=new Array("2","8","14","18","22","36");
var oSetNineIds=new Array("49","50","51","52","53","54");

var oSetTen=new Array("5","23","36","40","43","45");
var oSetTenIds=new Array("55","56","57","58","59","60");

function CheckNums(x,z)
{
var i;
var y=document.getElementById("myform");
for(i=0;i<6;i++)
{
if(x == y.elements[0].value)
{
document.getElementById(z).style.color="red";
}
else if(x == y.elements[1].value)
{
document.getElementById(z).style.color="red";
}
else if(x == y.elements[2].value)
{
document.getElementById(z).style.color="red";
}
else if(x == y.elements[3].value)
{
document.getElementById(z).style.color="red";
}
else if(x == y.elements[4].value)
{
document.getElementById(z).style.color="red";
}
else if(x == y.elements[5].value)
{
document.getElementById(z).style.color="red";
}
else
{
document.getElementById(z).style.color="black";
}
}
}
</script>
<style type="text/css" media="screen">
body
{
font-family:Arial;
}
</style>
</head>

<body>

<h3>Winning Numbers</h3>

<form name="myform" id="myform" title="A Little Form">
<input type="text" name="fieldone" size="1" maxlength="2" />

&nbsp;
<input type="text" name="fieldtwo" size="1" maxlength="2" />
&nbsp;
<input type="text" name="fieldthree" size="1" maxlength="2" />
&nbsp;
<input type="text" name="fieldfour" size="1" maxlength="2" />
&nbsp;
<input type="text" name="fieldfive" size="1" maxlength="2" />
&nbsp;

<input type="text" name="fieldsix" size="1" maxlength="2" />
</form>

<h3>Your Numbers</h3>

<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table1">
<tr>
<td width="100"><b>Set One</b></td>
<td width="35" id="1">2</td>
<td width="35" id="2">7</td>

<td width="35" id="3">13</td>
<td width="35" id="4">17</td>
<td width="35" id="5">33</td>
<td width="35" id="6">47</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetOne,oSetOneIds)" /></td>
</tr>
</table>

<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table2">
<tr>
<td width="100"><b>Set Two</b></td>
<td width="35" id="7">12</td>
<td width="35" id="8">19</td>
<td width="35" id="9">22</td>
<td width="35" id="10">31</td>

<td width="35" id="11">32</td>
<td width="35" id="12">41</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetTwo,oSetTwoIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table3">
<tr>
<td width="100"><b>Set Three</b></td>

<td width="35" id="13">17</td>
<td width="35" id="14">22</td>
<td width="35" id="15">23</td>
<td width="35" id="16">25</td>
<td width="35" id="17">26</td>
<td width="35" id="18">42</td>

<td><input type="submit" name="submit" id="submit" value="Check"
onClick="CheckNums(oSetThree,oSetThreeIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table4">
<tr>
<td width="100"><b>Set Four</b></td>
<td width="35" id="19">14</td>
<td width="35" id="20">21</td>

<td width="35" id="21">28</td>
<td width="35" id="22">29</td>
<td width="35" id="23">37</td>
<td width="35" id="24">46</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetFour,oSetFourIds)" /></td>
</tr>
</table>

<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table5">
<tr>
<td width="100"><b>Set Five</b></td>
<td width="35" id="25">1</td>
<td width="35" id="26">5</td>
<td width="35" id="27">6</td>
<td width="35" id="28">15</td>

<td width="35" id="29">18</td>
<td width="35" id="30">35</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetFive,oSetFiveIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table6">
<tr>
<td width="100"><b>Set Six</b></td>

<td width="35" id="31">10</td>
<td width="35" id="32">11</td>
<td width="35" id="33">31</td>
<td width="35" id="34">36</td>
<td width="35" id="35">38</td>
<td width="35" id="36">48</td>

<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetSix,oSetSixIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table7">
<tr>
<td width="100"><b>Set Seven</b></td>
<td width="35" id="37">3</td>
<td width="35" id="38">4</td>

<td width="35" id="39">15</td>
<td width="35" id="40">23</td>
<td width="35" id="41">36</td>
<td width="35" id="42">49</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetSeven,oSetSevenIds)" /></td>
</tr>
</table>

<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table8">
<tr>
<td width="100"><b>Set Eight</b></td>
<td width="35" id="43">1</td>
<td width="35" id="44">3</td>
<td width="35" id="45">27</td>
<td width="35" id="46">36</td>

<td width="35" id="47">39</td>
<td width="35" id="48">42</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetEight,oSetEightIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table9">
<tr>
<td width="100"><b>Set Nine</b></td>

<td width="35" id="49">2</td>
<td width="35" id="50">8</td>
<td width="35" id="51">14</td>
<td width="35" id="52">18</td>
<td width="35" id="53">22</td>
<td width="35" id="54">36</td>

<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetNine,oSetNineIds)" /></td>
</tr>
</table>
<br />
<table cellpadding="5" cellspacing="0" border="1" style="border-
collapse:collapse;" id="table10">
<tr>
<td width="100"><b>Set Ten</b></td>
<td width="35" id="55">5</td>
<td width="35" id="56">23</td>

<td width="35" id="57">36</td>
<td width="35" id="58">40</td>
<td width="35" id="59">43</td>
<td width="35" id="60">45</td>
<td><input type="submit" name="submit" id="submit"
value="Check" onClick="CheckNums(oSetTen,oSetTenIds)" /></td>
</tr>
</table>

</body>
</html>
 
J

Jeremy J Starcher

Hello,
I am not a good programmer. I wrote a javascript to check my lotto
numbers. I would like feedback on how I could make the script I wrote
'better'. Thanks in advance, I really look forward to seeing different
solutions to the script I wrote. I am sure there there is a much better
way to accomplish the end result.
Click to expand...


I don't understand the logic of WHAT the script should do or why,
it might be obvious to lottery players though.

I like to embed documentation into what I do because I suffer from
a bad case of CRS syndrome.

Here is the code:

<html>
<head>
<title>Check Lotto Nums</title>
<script type="text/javascript">
Click to expand...

A step ahead of most folks! A valid script declration.
var oSetOne=new Array("2","7","13","17","33","47");
var oSetOneIds=new Array("1","2","3","4","5","6");
Click to expand...

These should be numbers, not strings. I'll address that below.

I don't know what the connection is between oSetOne and oSetOneIds.
Could the second be calculated from the former?

In addition, a 2D array would work better than your current arrangement


var myNums = [
[2,7,13,17,33,47],
[12,19,22,31,32,41],
<snip>
[5,23,36,40,43,45]
];

By using JSON (object notation) you could store all the data together in
a single object -- but thats a touch more advanced and wouldn't worry
about right now.

function CheckNums(x,z)
{
var i;
Click to expand...

Local variables. That's good.
var y=document.getElementById("myform");
Click to expand...

Excellent, not calling 'document.getElementById("myform")' repeatedly. A
better variable name would work.
for(i=0;i<6;i++)
Click to expand...

Magic number! (6). You can work out the end of the array if you do it
that way.

{
if(x == y.elements[0].value)
Click to expand...


If you switch the above from strings to numbers, then you'll want this
syntax:

if(x === +y.elements[0].value)

Or, easier read:
if(x === +(y.elements[0].value))


The '===' matches type and unary + does a conversion to a number.


No need to repeat this over and over, BTW:
if(x === +(y.elements[ i ].value))
^^^^^^
document.getElementById(z).style.color="red";
Click to expand...


Better to specify the RGB. Not all UAs will decode color names.
<style type="text/css" media="screen"> body
{
font-family:Arial;
}
</style>
</head>

<body>
Click to expand...


You repeat much of the same information in the tables below as you do in
your arrays above. You could auto-generate that display from your arrays
and not duplicate things.



I'd find another way to display things besides a table, but I didn't give
a lot of thought about your markup.

Include a DOCTYPE. HTML 4.01 strict. (Don't use XHTML.) Specify widths
in your CSS and be sure to include units. You specify "35" a lot, but 35
WHAT?

Best practices say to specify sizes either in em's or percentages.
 
H

HughPugh

I don't understand the logic of WHAT the script should do or why,
it might be obvious to lottery players though.

I like to embed documentation into what I do because I suffer from
a bad case of CRS syndrome.
Click to expand...

Reply:
Thank you very much! I now understand that a quoted array member is a
string type, not number type. I didn't know that...

I very much appreciate your feedback. I should have given a
description of what the script is doing for the user(me). I put in the
winning lotto nums in the text input, and then if I have a matching
number in one of my sets, the matching number is set to
style.color="red";

Thanks again!
 
T

Thomas 'PointedEars' Lahn

Jeremy said:
I don't understand the logic of WHAT the script should do or why,
it might be obvious to lottery players though.

I like to embed documentation into what I do because I suffer from
a bad case of CRS syndrome.



A step ahead of most folks! A valid script declration.

Local variables. That's good.
Click to expand...

However, since `i' is only used as the iterator value,
it would ease maintenance to declare it there:

for (var i = ...; ...; ...)
These should be numbers, not strings.
Nonsense.

I'll address that below.
Click to expand...

Me too.
{
if(x == y.elements[0].value)
Click to expand...


If you switch the above from strings to numbers, then you'll want this
syntax:

if(x === +y.elements[0].value)

Or, easier read:
if(x === +(y.elements[0].value))


The '===' matches type and unary + does a conversion to a number.
Click to expand...


`+' does convert a value to a number, however it not only converts
representations of decimal values to number. Therefore, parseInt(..., 10)
is more reliable here.

However, using strings for both the values to compare against and foregoing
type conversion entirely is probably most efficient here.
[...]
document.getElementById(z).style.color="red";
Click to expand...


Better to specify the RGB. Not all UAs will decode color names.
Click to expand...


Then those UAs are broken and don't deserve to be supported.
`red' is a color value that is specified since CSS 1.

Here is a real problem for a change. The last item of a `font-family'
property value should be a generic font-family, to take care of the
event that a font with the listed names is not available.


PointedEars
 
H

HughPugh

I don't understand the logic of WHAT the script should do or why,
it might be obvious to lottery players though.
Click to expand...
I like to embed documentation into what I do because I suffer from
a bad case of CRS syndrome.
Click to expand...
A step ahead of most folks! A valid script declration.
Click to expand...
Local variables. That's good.
Click to expand...

However, since `i' is only used as the iterator value,
it would ease maintenance to declare it there:

for (var i = ...; ...; ...)
These should be numbers, not strings.
Nonsense.

I'll address that below.
Click to expand...

Me too.
{
if(x == y.elements[0].value)
Click to expand...
Click to expand...

If you switch the above from strings to numbers, then you'll want this
syntax:
Click to expand...
if(x === +y.elements[0].value)
Click to expand...

Or, easier read:
if(x === +(y.elements[0].value))
Click to expand...

The '===' matches type and unary + does a conversion to a number.
Click to expand...

`+' does convert a value to a number, however it not only converts
representations of decimal values to number. Therefore, parseInt(..., 10)
is more reliable here.

However, using strings for both the values to compare against and foregoing
type conversion entirely is probably most efficient here.
[...]
document.getElementById(z).style.color="red";
Click to expand...
Click to expand...

Better to specify the RGB. Not all UAs will decode color names.
Click to expand...

Then those UAs are broken and don't deserve to be supported.
`red' is a color value that is specified since CSS 1.

Here is a real problem for a change. The last item of a `font-family'
property value should be a generic font-family, to take care of the
event that a font with the listed names is not available.

PointedEars
Click to expand...


All points taken and appreciated greatly! Please do understand, I use
this script for my own personal benefit at this time. If I were
posting this on a website for consumption by the masses, I would have
specified a doctype, and I would have definitely ensured that my font-
family included a generic serif or sans-serif declaration.

:D

Thanks so much for the feedback!
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Can someone tell me if this a real tracker? Or is it one designed to show you a different message at certain times, ie. acting like one? 0
Nested Loop Insert Page Break 1
Javascript DOM 1
Image shifts to the right when export the page to pdf 4
How can I calculate the last payment of the year to be the sum of all previous payments for that year and subtracting it from Research Costs value? 7
Can anyone please help? HTML - two tables applying different styles 4
Script to send email not working 1
Uncaught ReferenceError: item is not defined at HTMLButtonElement.onclick in the: <button onclick="item.inserir()">Inserir dados</button> 1

Staff online

Members online

Total: 59 (members: 2, guests: 57)
Robots: 473

Forum statistics

Threads
473,995
Messages
2,570,230
Members
46,816
Latest member
SapanaCarpetStudio

Latest Threads

Top