Looking for feedback on Matching Exercises Maker/ Builder

Discussion in 'Javascript' started by lorlarz, Aug 11, 2008.

  1. lorlarz

    lorlarz Guest

    Looking for feedback on Matching Exercises Maker/ Builder:

    http://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.

    I also worry about using the ul as the container for the whole thing.

    Kind feedback would be appreciated (I am a little tired of harsh
    feedback from newsgroups).

    Thanks.
    lorlarz, Aug 11, 2008
    #1
    1. Advertising

  2. lorlarz wrote:
    > Looking for feedback on Matching Exercises Maker/ Builder:
    >
    > http://mynichecomputing.com/ReadIt/translateT.html


    <http://validator.w3.org/check?uri=http%3A%2F%2Fmynichecomputing.com%2FReadIt%2FtranslateT.html&ss=1&group=0&verbose=1>
    <http://jigsaw.w3.org/css-validator/validator?profile=css2&warning=2&uri=http%3A%2F%2Fmynichecomputing.com%2FReadIt%2FtranslateT.html>
    <http://www.w3.org/QA/Tips/color>

    > 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
    --
    Prototype.js was written by people who don't know javascript for people
    who don't know javascript. People who don't know javascript are not
    the best source of advice on designing systems that use javascript.
    -- Richard Cornford, cljs, <f806at$ail$1$>
    Thomas 'PointedEars' Lahn, Aug 11, 2008
    #2
    1. Advertising

  3. lorlarz

    lorlarz Guest

    I like a lot of your feedback.

    On Aug 11, 1:00 pm, Thomas 'PointedEars' Lahn <>
    wrote:
    > lorlarz wrote:
    > > Looking for feedback on Matching Exercises Maker/ Builder:

    >
    > >http://mynichecomputing.com/ReadIt/translateT.html

    >
    > <http://validator.w3.org/check?uri=http%3A%2F%2Fmynichecomputing.com%2....>
    > <http://jigsaw.w3.org/css-validator/validator?profile=css2&warning=2&u...>
    > <http://www.w3.org/QA/Tips/color>
    >
    > > 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 am considering just a little extra code (including one more array),
    so this
    is not a problem.

    >
    > 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"
    >     }
    >   };
    >


    While that seems like a good suggestion, note that the application
    takes in
    a JSON array to _swap out_ sets. Can you write this to be a valid
    part
    of such a 5 piece JSON array of objects?

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



    Yes, 2 UL elements would certainly provide an easy fix. Will do.

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


    Actually, the code deals with either attachEvent and addEvent (look
    more
    closely).


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


    The code retrys if an ajax call fail, so there is a reason for the way
    it is.

    > - ISTM you do not need XHR here, see above.
    > - You use `innerHTML', which is proprietary and error-prone, needlessly.
    >


    All say innerHTML is very, very cross-browser.

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


    You are generally right (when you do not miss a feature or two or
    three),
    so thanks.

    >
    > > 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
    > --
    > Prototype.js was written by people who don't know javascript for people
    > who don't know javascript. People who don't know javascript are not
    > the best source of advice on designing systems that use javascript.
    >   -- Richard Cornford, cljs, <f806at$ail$1$>
    lorlarz, Aug 11, 2008
    #3
  4. lorlarz

    lorlarz Guest

    On Aug 11, 1:45 pm, lorlarz <> wrote:
    > I like a lot of your feedback.
    >
    > On Aug 11, 1:00 pm, Thomas 'PointedEars' Lahn <>
    > wrote:
    >
    >
    >
    > > lorlarz wrote:
    > > > Looking for feedback on Matching Exercises Maker/ Builder:

    >
    > > >http://mynichecomputing.com/ReadIt/translateT.html

    >
    > > <http://validator.w3.org/check?uri=http%3A%2F%2Fmynichecomputing.com%2...>
    > > <http://jigsaw.w3.org/css-validator/validator?profile=css2&warning=2&u...>
    > > <http://www.w3.org/QA/Tips/color>

    >
    > > > 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 am considering just a little extra code (including one more array),
    > so this
    > is not a problem.
    >


    I did make the change, so now any value for the "known" words (kwords)
    should work.
    className no longer holds the correct answers values in the lower ul.

    >
    >
    > > 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"
    > > }
    > > };

    >
    > While that seems like a good suggestion, note that the application
    > takes in
    > a JSON array to _swap out_ sets. Can you write this to be a valid
    > part
    > of such a 5 piece JSON array of objects?
    >


    I am still interested about this, but my randomization works fine.

    [snip]

    > > > I also worry about using the ul as the container for the whole thing.

    >
    > > It might be better to use two UL elements, indeed.

    >
    > Yes, 2 UL elements would certainly provide an easy fix. Will do.
    >


    And, indeed I did it. I now use 2 lists.

    >
    >
    >



    [snip]


    >
    > > That should be enough homework for today.



    >
    > You are generally right (when you do not miss a feature or two or
    > three),
    > so thanks.
    >
    >
    >
    > > > 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/>

    >


    Hey, if you guys are so smart, then why am I the first one to build
    both a universal
    JS automatic drag and drop maker and the first automatic multi-
    featured JS matching
    activity maker? This is what I would like to know.
    lorlarz, Aug 11, 2008
    #4
  5. lorlarz

    lorlarz Guest

    On Aug 11, 1:00 pm, Thomas 'PointedEars' Lahn <>
    wrote:
    > lorlarz wrote:
    > > Looking for feedback on Matching Exercises Maker/ Builder:

    >
    > >http://mynichecomputing.com/ReadIt/translateT.html

    >
    > <http://validator.w3.org/check?uri=http%3A%2F%2Fmynichecomputing.com%2...>
    > <http://jigsaw.w3.org/css-validator/validator?profile=css2&warning=2&u...>
    > <http://www.w3.org/QA/Tips/color>

    [snip]
    >
    > > 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



    Dear PointedEars
    I took what feedback of yours I could. I took care of the list
    problem and have 2 lists.
    I took action against the 1.0 problem. I made less global. I made
    arrays with [] . I store
    correct answers without using classNames. I also added some
    flexibility to the program.
    And, I took care of an unnoticed bug (which I won't bring up; but it
    was stupid).

    Your feedback again or that of others would be appreciated. Thanks
    already. And,
    thanks again in advance.
    lorlarz, Aug 14, 2008
    #5
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. DobySoft
    Replies:
    0
    Views:
    442
    DobySoft
    Apr 28, 2004
  2. DobySoft
    Replies:
    0
    Views:
    597
    DobySoft
    Nov 14, 2004
  3. Michael

    pix maker dosen't work

    Michael, Jun 2, 2004, in forum: HTML
    Replies:
    8
    Views:
    717
    Michael
    Jun 2, 2004
  4. Phlip
    Replies:
    5
    Views:
    556
    Stefan Behnel
    Jan 13, 2010
  5. Michael Schuerig

    State machine builder: looking for feedback

    Michael Schuerig, Dec 30, 2007, in forum: Javascript
    Replies:
    0
    Views:
    192
    Michael Schuerig
    Dec 30, 2007
Loading...

Share This Page