el*********@ele ctrician.com wrote:
The problem was that I searched an array of conductor ampacities, but
when the end of the array was reached and the right size conductor
could not be found because the derating was too high I needed to stop
the program and alert the user to enter smaller values and fill all the
output form values with blanks and terminate the program. I
accomplished this by setting a flag to True and broke out of the outer
loop with break loop then filled in the blanks by running the
endprogram when the flag is true. It works just fine, now. This is a
rather long program and I have obfuscated it, but if you want to see it
work it is at,
http://electrician.com/calculators/wireocpd_ver_1.html
I will not pretend that I fully understand what you are talking about, since
I am not an electrician, and your explanation lacks specific reference to
the code you are using, and again, to what you are referring to (will you
learn to quote?). However, reviewing your code, it is obvious to me that
it (still) has a number of flaws.
- The underlying HTML code is not Valid. You cannot expect client-side
script code to work on not Valid markup, as you cannot expect a house
to stand on quicksand.
<URL:http://validator.w3.or g/check?uri=http://electrician.com/calculators/wireocpd_ver_1. html>
- The script is quite large. For HTTP request efficiency, large resources
should be splitted into smaller chunks. This means the script it should
be moved to an external script resource, e.g. program.js, which should be
included in the HTML document with
<script type="text/javascript" src="program.js "></script>
See also:
<URL:http://www.websiteopti mization.com/services/analyze/wso.php?url=htt p://electrician.com/calculators/wireocpd_ver_1. html>
- ECMAScript implementations provide a boolean type, with predefined
values of `true' and `false'. It is quite inefficient and error-prone
(unrecognized typos!) to use the string values "true"/"True" and
"false"/"False" instead.
- for (var i = 0; i <= form.aN.length; i++)
{
if (form.aN[i].selected)
{
ay =(form.aN[i].value);
break;
}
}
is syntactically correct, but semantically wrong, as it loops also for
form.aN[form.aN.length], which does not exist because the collection's
index is zero-based. Using
for (var i = 0; i < form.aN.length; i++)
instead, solves this problem. However, it would be still inefficient
(because of repeated deep lookup) and error-prone (because of proprietary
referencing). Therefore, it should be
for (var i = 0, len = form.elements['aN'].options.length ; i < len; i++)
Because the reference `form.elements' is used more than one time, it
should be assigned to a local variable once and that variable should
be used instead:
var es = form.elements, o = es['aN'].options;
for (var i = 0, len = o.length; i < len; i++)
But "aN" refers to one single select element that does not have the
"multiple" attribute. Therefore, it should be simply
ay = o[o.selectedIndex].value;
_without_ any unnecessary loop. You will also observe here that the
parentheses in
ay =(form.aN[i].value);
were redundant. Since you have been using obfuscation (which I recommend
against) to make the code as short as possible, you should endeavour not
to include any unnecessary code.
- au = eval(form.S[i].value);
is nonsense. The global eval() method merely evaluates its argument.
Unless its argument is a _string_ value that contains an arithmetic
expression, eval() is entirely redundant, inefficient, and error-prone.
While "S" refers to a `select' element in your HTML code and the `value'
property of HTMLSelectEleme nt objects should not be relied upon (use
selectRef.optio ns[selectRef.selec tedIndex] instead), the value of
that property in your case could be either (e.g. for the first item)
"0" or "21-25 (70-77F)".
Assuming it would be "0", being the value of the `value' property of the
respective HTMLOptionEleme nt object as specified in W3C DOM Level 2 HTML,
+es['S'][i]
or even
parseInt(es['S'][i], 10)
would be more efficient.
<URL:http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-59351919>
Assuming that it would be instead "21-25 (70-77F)" as IIRC known from
some DOMs, eval(form.S[i].value) or its standards-compliant reference
equivalent would result in a syntax error.
Therefore the loop should be
for (var i = 0, o = es['S'].options, len = o.length; i < len; i++)
and it should be either
// allows for interpretation of 0x... as hexadecimal value
au = +o[i].value;
or
// parses 0x... as 0
au = parseInt(o[i].value, 10);
However, no loop is necessary at all here, too:
au = parseInt(o[o.selectedIndex].value, 10);
- for (var n = 0; n <= form.aY.length; n++)
{
if (form.aY[n].checked)
{
aB =(form.aY[n].value);
break;
}
}
is different from the above insofar it refers to a group of radiobuttons
with the same name, and not a `select' element. Therefore, the loop is
necessary here, indeed. However,
var aRadio = es['aY'];
for (var n = 0, len = aRadio.length; n < len; n++)
{
if (aRadio[n].checked)
{
aB = aRadio[n].value;
break;
}
}
is still better. (You will observe that the iterator variable and the
`len' variable can be reused. To avoid repeated declaration, you should
declare them locally before the `for' statements only once, and only
[re-]initialize them in each `for' statement. Certainly you do not need
two iterator variables `i' and `n' if the corresponding loops are
independent.)
You should consider using a method (function) that returns the value of
the selected radiobutton in a group. IIRC such a method can be found in
the newsgroup's FAQ, for example.
- As you will probably have guessed after having read the above,
bx =(eval((1.25 *(form.ab.value )))) +(eval(form.aS. value));
is utter nonsense, really.
- If you use boolean values as suggested,
if (ax == "True")
could be simplified to
if (ax)
and
if ((as == "False") &&(G == "False"))
could be simplified to
if (!as && !G)
and so on. (Whereas I recommend to avoid using identifiers starting with
a capital letter if they does not refer to a constructor or a factory;
just to avoid confusion, and to make source code easier readable.)
- ISTM other flaws/errors in your code include merely repetitions of the
flaws/errors that are described above.
HTH
PointedEars
