1
\$\begingroup\$

Here is a group of questions (hidden by default except the first one) with yes|no buttons

<div id="email" class="form-item">
        <h2>Q with 1 more option?</h2>
    <input type="button" value="true" data-value="web" />
    <input type="button" value="no" />
</div>
<div id="web" class="form-item">
        <h2>Q with 2 more option?</h2>
    <input type="button" value="yes" data-value="analy,seo" />
    <input type="button" value="no" />
</div>
<div id="analy" class="form-item">
        <h2>Q3?</h2>
    <input type="button" value="yes" data-value="qwe" />
    <input type="button" value="no" />
</div>
<div id="seo" class="form-item">
        <h2>Q4?</h2>
    <input type="button" value="yes" data-value="asd" />
    <input type="button" value="no" />
</div>

and the script to reveal the next relevant question (or multiple questions):

$('.form-item:nth-child(1)').slideDown('fast');

$('input[value="yes"],input[value="true"]').click(function () {
    next_id = $(this).attr('data-value');
    matches = [next_id];
    rx = /[^a-z|^0-9]/g;
    if (patt = next_id.match(rx)) {
        matches = next_id.split(patt[0]);
    }

    i = 0;
    while (matches[i]) {
        $('#' + matches[i]).slideDown('fast');
        i++;
    }
});

Sandbox: http://jsfiddle.net/eapo/pu53rx31/

There is any way to simplify this code?

\$\endgroup\$
2
  • \$\begingroup\$ Why introduce the complication of a regular expression? Why not simply have matches = next_id.split(',')? \$\endgroup\$
    – 76484
    Jan 4, 2015 at 4:05
  • \$\begingroup\$ @76484 because in my case i can use any other isolation character \$\endgroup\$
    – eaposztrof
    Jan 6, 2015 at 22:21

1 Answer 1

4
\$\begingroup\$

Comments:

The idea of splitting your data-values with different isolation characters seems silly, just using a comma seems much better, as @76484 suggested, and removing the Regular Expression altogether.

However stepping around that, I can see a few things you could fix up:

Yes and No buttons:

You use yes and no for testing the input, why not use true and false, that way instead of $('input[value="yes"],input[value="true"]'), you can just use $('input[value]').

<input type="button" value="true" data-value="foo" />
<input type="button" value="false" />

Regular Expression declaration:

Assuming entire removal of the regular expression isn't possible: you only use rx once, meaning you can just place the expression in the loop:

if (patt = next_id.match(/[^a-z|^0-9]/g)) {

While loops:

while loops are notorious for causing recursion to occur, so you ought to replace it with a for loop.

It also removes the need for two extra lines for declaring the variable and incrementing the variable.

for (var i = 0; matches[i]; i++) {
  $('#' + matches[i]).slideDown('fast');
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.