1
\$\begingroup\$

Hi friends i have this code for select input change, i need some good advice if this code is ok like that or is a way to optimize this code? thanks!

This code is used for changing info's between selected input options.

This code is for two select inputs "SECTIONS" and "AGE CATEGORY". If a user select a value from sections the age category will be changed regarding the value from Section option.

Example: If i select from sections value "Solo" the Age category values will be changed to values for "Solo" such as (4-6 Years, 7-9 Years,and so on) Script is used for a dancing contest registration form.

javascript:

$("select[name=sections]").on("change", function () {
   if ($(this).val() === 'Solo') {
      $('#mini,#copii,#juniori').hide();
          $('#baby,#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15,#seniori').show();
          $("input[type='text'][name='numeformatie']").val('-');
          $("input[type='text'][name='numeformatie']").prop('disabled', true); 
          $("input[type='text'][name='nrdansatori']").val('1');
          $("input[type='text'][name='dansator1']").val('');
          $(".dansator1").show();
          $("input[type='text'][name='nrdansatori']").prop('disabled', true);} 
        else if ($(this).val() === 'Duo/Trio/Quartet') {
          $('#baby,#mini,#copii,#juniori,#seniori').show();
          $('#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15').hide();
          $("input[type='text'][name='numeformatie']").val('-');
          $("input[type='text'][name='numeformatie']").prop('disabled', true);
          $("input[type='text'][name='nrdansatori']").val();
          $("input[type='text'][name='dansator1']").val('');
          $(".dansator1").show();
          $("input[type='text'][name='nrdansatori']").prop('disabled', false);}
        else if ($(this).val() === 'Grupuri') {
          $('#baby,#mini,#copii,#juniori,#seniori').show();
          $('#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15').hide();
          $("input[type='text'][name='numeformatie']").val();
          $("input[type='text'][name='numeformatie']").prop('disabled', false);
          $("input[type='text'][name='dansator1']").val('-');
          $(".dansator1").hide();}
        else if ($(this).val() === 'Formatii') {
          $('#baby,#mini,#copii,#juniori,#seniori').show();
          $('#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15').hide();
          $("input[type='text'][name='numeformatie']").val('');
          $("input[type='text'][name='numeformatie']").prop('disabled', false);
          $("input[type='text'][name='dansator1']").val('-');
          $("input[type='text'][name='nrdansatori']").prop('disabled', false);
          $(".dansator1").hide();}
    }).trigger('change');

Html:

<select name="sections" id="sections<?php echo $id;?>" class="form-control">
                <option class="noselect" selected value="<?php echo $sectiuni; ?>"><?php echo $sectiuni; ?></option>
                <option id="solo" value="Solo">Solo</option>
                <option id="duo" value="Duo/Trio/Quartet">Duo/Trio/Quartet</option>
                <option id="grupuri" value="Grupuri">Grupuri 5-12 dansatori</option>
                <option id="formatii" value="Formatii">Formaţii peste 13 dansatori</option>
              </select>
 <select id="agecategory<?php echo $id;?>" name="agecategory" class="form-control">
            <option class="noselect" selected value="<?php echo $catvarsta; ?>"><?php echo $catvarsta; ?></option>
            <option id="baby" value="4-6 ani - BABY">4-6 years</option>
            <option id="mini" value="7-9 ani - MINI">7-9 years</option>
            <option id="solo7" value="7 ani">7 years - SOLO</option>
            <option id="solo8" value="8 ani">8 years - SOLO</option>
            <option id="solo9" value="9 ani">9 years- SOLO</option>
            <option id="copii" value="10-12 ani - COPII">10-12 years</option>
            <option id="solo10" value="10 ani">10 years - SOLO</option>
            <option id="solo11" value="11 ani">11 years- SOLO</option>
            <option id="solo12" value="12 ani">12 years- SOLO</option>
            <option id="juniori" value="13-15 ani - JUNIORI">13-15 years</option>
            <option id="solo13" value="13 ani">13 years - SOLO</option>
            <option id="solo14" value="14 ani">14 years - SOLO</option>
            <option id="solo15" value="15 ani">15 years - SOLO</option>
            <option id="seniori" value="+ 16 ANI - SENIORI">+ 16 years</option>
          </select>
\$\endgroup\$
  • \$\begingroup\$ You could consider using <optgroup>-elements, which would simplify the code as well. \$\endgroup\$ – morbusg Oct 22 '19 at 10:29
  • \$\begingroup\$ Can u show me how ? \$\endgroup\$ – Fido Oct 22 '19 at 15:48
2
\$\begingroup\$

Use proper indentation for better readability.

Hide all values first. This way you only need to keep track of which values you want shown.

You can use parallel arrays or an object to better keep track of which values you want shown/hidden. For example:

const selectionOptions = {
  "Solo": { 
    "optionsToShow": '#baby,#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15,#seniori'
  }
}

Here's a JSFiddle demonstration: https://jsfiddle.net/nxcpLaug/1/

You can do the same to list the values you want reset:

const selectionOptions = {
  "Solo": { 
    "optionsToShow": '#baby,#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15,#seniori'
    "elementsToResetValue": "#numeformatie,#numeformatie"
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your answer and support @dustytrash, i will test'it and i will come with a reply \$\endgroup\$ – Fido Oct 21 '19 at 8:27
  • 1
    \$\begingroup\$ It works just fine, thank you for taking your time to help me, im new and i have so much to learn from you guys. \$\endgroup\$ – Fido Oct 21 '19 at 8:46
1
\$\begingroup\$

This can only be an incomplete review, as the HTML is still missing, but it should point you in the direction of things to do.

You have a lot of repetition in your code, here's how to do it with less repetition.

var $num= $("input[type='text'][name='numeformatie']");
var $nrd= $("input[type='text'][name='nrdansatori']");
var sol='#solo7,#solo8,#solo9,#solo10,#solo11,#solo12,#solo13,#solo14,#solo15',
    cases={
      Solo:          {numv:'-',numdis:'true',nrdv:'1',dv1:'',nrddis:true,
                      shw:'.dansador1,#baby,#seniori,'+sol, hid:'#mini,#copii,#juniori'},
      'Duo/Trio/Quartet': {numv:'-',numdis:'true',nrdv:'1',dv1:'',nrddis:true,
                      shw:'.dansador1,#baby,#mini,#copii,#juniori,#seniori', hid:sol},
      Grupuri:       {numv:'-',numdis:'true',nrdv:'1',dv1:'',nrddis:true,
                      shw:'#baby,#mini,#copii,#juniori,#seniori', hid:'.dansador1,'+sol},
      Formatii:      {numv:'-',numdis:'true',nrdv:'1',dv1:'',nrddis:true,
                      shw:'#baby,#mini,#copii,#juniori,#seniori', hid:'.dansador1,'+sol},
};
$("select[name=sectiuni]").on("change", function () {
   var c=cases[$(this).val()];
   $(c.hid).hide();
   $(c.shw).show();
   $num.val(c.numv).prop('disabled',c.numdis);
   $nrd.val(c.nrdv).prop('disabled',c.nrddis);
}).trigger('change');
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

\$\endgroup\$
  • 4
    \$\begingroup\$ I'd say that this is not a review at all but a code dump. Please tell us something about your recommendations so that we may learn from your thought process. \$\endgroup\$ – 200_success Oct 20 '19 at 16:44
  • 1
    \$\begingroup\$ I didn't downvote, but I don't like your lack of naming standards. c is not very descriptive. \$\endgroup\$ – dustytrash Oct 20 '19 at 16:46
  • \$\begingroup\$ Fair enough. I am new to Code review and wanted to quickly sketch, how things can be done with less redundancy and repetition. But as said before, my time is running out now. \$\endgroup\$ – cars10m Oct 20 '19 at 16:47
  • 2
    \$\begingroup\$ @cars10m Simply adding "You have a lot of repetition in your code, here's how to do it with less repetition" would make this answer better. I will be kind and edit it in for you this time. \$\endgroup\$ – Simon Forsberg Oct 20 '19 at 19:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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