JS Code Advice (greatly appreciated)

Discussion in 'Javascript' started by rupoopin, Feb 22, 2008.

  1. rupoopin

    rupoopin Guest

    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:

    <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");
    if(x == y.elements[0].value)
    else if(x == y.elements[1].value)
    else if(x == y.elements[2].value)
    else if(x == y.elements[3].value)
    else if(x == y.elements[4].value)
    else if(x == y.elements[5].value)
    <style type="text/css" media="screen">


    <h3>Winning Numbers</h3>

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

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

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

    <h3>Your Numbers</h3>

    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table1">
    <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>

    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table2">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table3">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table4">
    <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>

    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table5">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table6">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table7">
    <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>

    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table8">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table9">
    <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>
    <br />
    <table cellpadding="5" cellspacing="0" border="1" style="border-
    collapse:collapse;" id="table10">
    <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>

    rupoopin, Feb 22, 2008
    1. Advertisements

  2. 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.
    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 = [

    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.

    Local variables. That's good.
    Excellent, not calling 'document.getElementById("myform")' repeatedly. A
    better variable name would work.
    Magic number! (6). You can work out the end of the array if you do it
    that way.

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

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

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

    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

    Best practices say to specify sizes either in em's or percentages.
    Jeremy J Starcher, Feb 23, 2008
    1. Advertisements

  3. rupoopin

    HughPugh Guest

    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

    Thanks again!
    HughPugh, Feb 23, 2008
  4. *nods*

    Understanding types in Javascript can be a hurdle for someone getting
    into it. Because of automatic type conversion, Javascript sometimes
    *appears* typeless.

    This might be of some use:
    Jeremy J Starcher, Feb 23, 2008
  5. However, since `i' is only used as the iterator value,
    it would ease maintenance to declare it there:

    for (var i = ...; ...; ...)
    Me too.

    `+' 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.

    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.

    Thomas 'PointedEars' Lahn, Feb 24, 2008
  6. rupoopin

    HughPugh Guest

    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.


    Thanks so much for the feedback!
    HughPugh, Feb 24, 2008
    1. Advertisements

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 (here). After that, you can post your question and our members will help you out.