2
\$\begingroup\$

I ask for your help to improve the form I created, keeping in mind that I am not a great expert in JavaScript programming.

I need to create a hotel search engine, with the possibility to choose the total number of rooms. Depending on the number of rooms selected, as many hidden form fields will be displayed, containing additional selection fields.

To make the idea of ​​what I have created better, I attach a screen.

enter image description here

To view the hidden fields in relation to the number of rooms chosen, I used this JavaScript code, repeated 5 times (maximum number of rooms). Is there an alternative or less complex code to create hidden input forms?

function admSelectCheck(nameSelect)
{
console.log(nameSelect);
if(nameSelect){
    admOptionValue = document.getElementById("room1").value;
    if(admOptionValue == nameSelect.value){
        document.getElementById("pax_room_1").style.display = "block";
    }
    else{
        document.getElementById("pax_room_1").style.display = "none";
    }
}
else{
    document.getElementById("pax_room_1").style.display = "none";
}

To realize the options for choosing each room, I used this JavaScript code repetition in proportion to the room number. That is:

  • 1 room -> entered code 1 once;
  • 2 rooms -> entered the code 2 times; etc.

function childSelect(nameSelect)
{
console.log(nameSelect);
if(nameSelect){
    admOptionValue = document.getElementById("child1").value;
    if(admOptionValue == nameSelect.value){
        document.getElementById("agechild1").style.display = "block";
    }
    else{
        document.getElementById("agechild1").style.display = "none";
    }
 }
 else{
    document.getElementById("agechild1").style.display = "none";
 }

 if(nameSelect){
    admOptionValue = document.getElementById("child2").value;
    if(admOptionValue == nameSelect.value){
        document.getElementById("agechild2").style.display = "block";
    }
    else{
        document.getElementById("agechild2").style.display = "none";
    }
}
else{
    document.getElementById("agechild2").style.display = "none";
}

if(nameSelect){
    admOptionValue = document.getElementById("child3").value;
    if(admOptionValue == nameSelect.value){
        document.getElementById("agechild3").style.display = "block";
    }
    else{
        document.getElementById("agechild3").style.display = "none";
    }
}
else{
    document.getElementById("agechild3").style.display = "none";
}
}

$(function(){
     $(':submit').click(function(){
          $('select').each(function(){
              if ( $(this).val() == '' )
              {
                  $(this).remove(); // or 
$(this).attr('disabled','disabled');
              }
          });
     });
});

While, I used this HTML code repeated the same way.

<div class="container_hidden">


    <div id="pax_room_1" style="display:none;" class="row_hidden">

        <div class="nrRoom" style="width: 100px;background: #f5a445;font-size: 15px;text-align: center;height: 30px; padding-top: 5px; border-radius: 10px 10px 0px 0px;"><font color="#000"><img src="images/family-room.png" alt="Icon Family Room" style="width: 19px;" /> Room 1</font></div>

            <div class="column"><font color="#000">Adult (18+)</font>
                <select id="adulti" name="nrAdult">
                    <option value="">-</option>
                    <option value="1">1</option>
                    <option value="2">2</option>
                    <option value="3">3</option>
                    <option value="4">4</option>
                    <option value="5">5</option>
                    <option value="6">6</option>        
                </select>
            </div>          
            <div class="column"><font color="#000">Children (0-17)</font>
                <select id="nrchild" name="nrChild" onchange="childSelect(this);">
                    <option value="">-</option>
                    <option id="child1" value="1">1</option>
                    <option id="child2" value="2">2</option>
                    <option id="child3" value="3">3</option>
                </select>
            </div>

                        <!-- Select Bambino 1 -->
                        <div id="agechild1" style="display:none;" class="row_hidden">
                        <div class="column"><font color="#000"> Child Age 1 </font>
                        <select id="child1" name="ageChild1">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        </div>

                        <!-- Select Bambino 1-2 -->
                        <div id="agechild2" style="display:none;" class="row_hidden">
                        <div class="column"><font color="#000"> Child Age 1 </font>
                        <select id="child1" name="ageChild1">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        <div class="column"><font color="#000"> Child Age 2 </font>
                        <select id="child2" name="ageChild2">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        </div>

                        <!-- Select Bambino 1-2-3 -->
                        <div id="agechild3" style="display:none;" class="row_hidden">
                        <div class="column"><font color="#000"> Child Age 1 </font>
                        <select id="child1" name="ageChild1">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        <div class="column"><font color="#000"> Child Age 2 </font>
                        <select id="child2" name="ageChild2">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        <div class="column"><font color="#000"> Child Age 3 </font>
                        <select id="child3" name="ageChild3">
                        <option value="">-</option>
                        <option value="1">1</option>
                        <option value="2">2</option>
                        <option value="3">3</option>
                        <option value="4">4</option>
                        <option value="5">5</option>
                        <option value="6">6</option>
                        <option value="7">7</option>
                        <option value="8">8</option>
                        <option value="9">9</option>
                        <option value="10">10</option>
                        <option value="11">11</option>
                        <option value="12">12</option>
                        <option value="13">13</option>
                        <option value="14">14</option>
                        <option value="15">15</option>
                        <option value="16">16</option>
                        <option value="17">17</option>
                        </select>
                        </div>
                        </div>
    </div>

I realize that it is too laborious, even if it works. This is why I ask you if there is a way to lighten the code.

\$\endgroup\$
  • \$\begingroup\$ where is admSelectCheck() called? I presume it is on the change event for the select list element with the number of rooms? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Dec 6 '17 at 22:00
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to give it a different title if there is something more appropriate. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Dec 6 '17 at 23:46
  • \$\begingroup\$ you say that you use the above HTML repeatedly to generate each row if I understand correctly, how come you are not keeping the id of the child1 child2 and so on ... not unique and your code still works? i mean the method childSelect(); is called via <select id="nrchild" onchange event and there you are using document.getELementById to get the child1 child2 child3` ? \$\endgroup\$ – Muhammad Omer Aslam Dec 8 '17 at 0:55
1
\$\begingroup\$

Abstracting common code to a function

One could pull out the common code to a common method. The name suggested below could perhaps be improved but at least it describes the task.

function updateDependentElementBasedonMainElement(selectedValue, mainElement, dependentElement) {
  if(selectedValue){
    admOptionValue = mainElement.value;
    if(admOptionValue == selectedValue){
        dependentElement.style.display = "block";
    }
    else{
        dependentElement.style.display = "none";
    }
 }
 else{
    dependentElement.style.display = "none";
 }
}

Then the function childSelect() can be simplified greatly, calling that function once for each pair of elements:

function childSelect(nameSelect) {
    updateDependentElementBasedonMainElement(nameSelect.value, document.getElementById("child1"), document.getElementById("agechild1"));
    updateDependentElementBasedonMainElement(nameSelect.value, document.getElementById("child2"), document.getElementById("agechild2"));
    updateDependentElementBasedonMainElement(nameSelect.value, document.getElementById("child3"), document.getElementById("agechild3"));
}

Caching DOM references

Instead of looking up the elements in the DOM each time, those can be stored in variables (and actually they could be referenced via the id attribute since those are defined, but some might argue that is poor form).

var child1 = document.getElementById("child1");
var agechild1 = document.getElementById("agechild1");
//etc. for other elements

Actually, const could be used instead of var (unless Browser compatibilty is an issue (e.g. if you are supporting users with IE 10 or earlier).

Using jQuery

It appears that your code utilizes jQuery (based on that last block in the Javascript section). Because that is available, the DOM lookups can be written in a shorter form - e.g.

var child1 = $("#child1");
var agechild1 = $("#agechild1");
//etc. for other elements

But if that last block is all that jQuery is being used for, you might consider replacing it with vanilla Javascript. For more information about evaluating jQuery's value, see You might not need jQuery.

This jQuery code:

$(function(){
   $(':submit').click(function(){
      $('select').each(function(){
          if ( $(this).val() == '' )
          {
              $(this).remove(); // or 
              $(this).attr('disabled','disabled');
          }
      });
 });

Appears to look for submit buttons (using the jQuery :submit selector - so you might have to look for elements matching input[type="submit"], button[type="submit"]) and adds a click handler to each. When the click handler runs, it removes any select list that doesn't have a value - and then apparently sets the disabled property to true.

\$\endgroup\$

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.