lorlarz said:
Looking for feedback on Matching Exercises Maker/ Builder:
http://mynichecomputing.com/ReadIt/translateT.html
<
http://validator.w3.org/check?uri=h...ReadIt/translateT.html&ss=1&group=0&verbose=1>
<
http://jigsaw.w3.org/css-validator/...//mynichecomputing.com/ReadIt/translateT.html>
For one thing, I am concerned about storing the matching kwork (known
word) as the className of the li which is the answer. I think this may
be problematic.
It is. CSS class names are restricted to a limited set of characters:
<
http://www.w3.org/TR/REC-CSS2/grammar.html#q1>
I suggest you use a mapping object instead:
var kwords = {
items: ["Shark", "Lion", "Dolphin", "Squirrel"],
map: {
Shark: "Tiburón",
Lion: "León",
Dolphin: "DelfÃn",
Squirrel: "Ardilla"
}
};
You can iterate over the `items' array to build the list (iterating over
`map' would use arbitrary order). Suppose `firstClicked' and `lastClicked'
store the words in the clicked list elements, then you can test whether
there is a match:
if (lastClicked === kwords.map[firstClicked])
{
// ...
}
else
{
// ...
}
I recommend not to bother your users with alert() messages (at least not
always); it indicates an error/problem where there might not be none and
drags the user's attention off from what was clicked, which is disturbing.
Use accessible Web-safe colors and at least one other style to indicate
correctness (or the opposite).
I also worry about using the ul as the container for the whole thing.
It might be better to use two UL elements, indeed.
As for your script:
- Why do you mix `new Array()' and `[]'?
- There are too many global variables.
- Math.random() needs a bugfix if it it returns 1.0.
- Your indentation sucks^W can be improved.
- You are using too many unnecessary parentheses around expressions
for my taste.
- You are not feature-testing anything, which is error-prone.
- You use identifiers for constructors to refer to simple objects.
- var i, item;
for (i = listEvents.length - 1; i >= 0; i = i - 1)
{
item = listEvents
;
can be rewritten more efficient as
for (var i = listEvents.length; i--
{
var item = listEvents;
- Your addEvent() is flawed, W3C's addEventListener() and MSHTML's
attachEvent() are not equivalent.
- There are too many lookups for the same reference, which makes this
inefficient and hard to maintain.
- Your getXHR() is error-prone, exception-handling is not supported in
all UAs that support XHR. The method is also needlessly inefficient:
you do not need advanced features of MSXML 3.0 here, so testing for
Microsoft.XMLHTTP suffices.
- Your handleReadyState() makes no sense to me: The callback assigned
to the `onreadystatechange' property is automatically called when the
readyState changes; you don't have to wait for that.
- ISTM you do not need XHR here, see above.
- You use `innerHTML', which is proprietary and error-prone, needlessly.
Search the newsgroup's archives for details. And try applying
<http://www.jslint.com/> to your code. (Some messages are
but a matter of taste, some are based on good recommendations.)
That should be enough homework for today.
Kind feedback would be appreciated (I am a little tired of harsh feedback
from newsgroups).
Well, you get what you paid for.
<http://jibbering.com/faq/>
PointedEars