how to make this javascript code shorter?

Question

I created this small game while ago and now i'm trying it find a way to make it script short and simpler i was wondering if there is any another way . I have four pics of animals and blow the pic i have four names . what the game suppose to do is when the users click on right image and right name both the image and the name has to disappear. Any help really appreciate it

function image_select() {
    var pic1 = document.getElementById("cow").value;
    var text1 = document.getElementById("cow_t").value;

    if (document.getElementById("cow").checked) {
        if (pic1 == text1) {
            var x = document.getElementById("cow_s");
            x.play();
        } else {
            alert("wrong selection");
        }
    } else {
        alert("no");
    }
}

function image_select2() {
    var pic2 = document.getElementById("dog").value;
    var text2 = document.getElementById("dog_t").value;

    if (document.getElementById("dog").checked) {
        if (pic2 == text2) {
            var x = document.getElementById("dog_s");
            x.play();
        } else {
            alert("wrong selection");
        }
    } else {
        alert("no");
    }

}


function image_select3() {
    var pic3 = document.getElementById("horse").value;
    var text3 = document.getElementById("horse_t").value;

    if (document.getElementById("horse").checked) {
        if (pic3 == text3) {
            var x = document.getElementById("horse_s");
            x.play();
        } else {
            alert("wrong selection");
        }
    } else {
        alert("no");
    }

}

function image_select4() {
    var pic4 = document.getElementById("pig").value;
    var text4 = document.getElementById("pig_t").value;

    if (document.getElementById("pig").checked) {
        if (pic4 == text4) {
            var x = document.getElementById("pig_s");
            x.play();
        } else {
            alert("wrong selection");
        }
    } else {
        alert("no");
    }

}

<div style="margin-left:230px;">
    <br>
    <br>
<img onmousedown="dog.play()" height="142" width="142" src="image/cow.jpg" alt="Cow" data-value="cow" onclick="selectImage(event)" />
 <img  height="142" width="142" src="image/dog.jpg" alt=""  data-value="dog" onclick="selectImage(event)"/> 
  <img height="142" width="142" src="image/horse.jpg" alt="" data-value="horse" onclick="selectImage(event)"/>  
    <img height="142" width="142"  src="image/pig.jpg" data-value="pig" onclick="selectImage(event)"/>
</div>
    <br>
    <br>
    <br>
    
    <div style="margin-left:400px;">
     <button type="button" onclick="selectName(event)" value="pig" id="pig_t">Cochon</button>
     <button type="button" onclick="selectName(event)" value="cow" id="cow_t">Vache</button>
     <button type="button" onclick="selectName(event)" value="horse" id="horse_t">Cheval</button>
     <button type="button" onclick="selectName(event)" value="dog" id="dog_t">chien</button>
    </div>

this should probably be on codereview

Alnitak
– Alnitak

2017-09-30 20:37:42 +00:00
Commented Sep 30, 2017 at 20:37 — Alnitak
– Alnitak, Commented Sep 30, 2017 at 20:37

Samuel Caillerie · Accepted Answer · 2017-09-30 20:35:20Z

1

Just put the name of the animal to the function (in fact, you seem to already does so, since you pass the event to selectName, but I don't see this function in your code) :

function image_select_animal(prefix) {
  var pic = document.getElementById(prefix).value;
  var text = document.getElementById(prefix + "_t").value;

  if (document.getElementById(prefix).checked) {
    if (pic == text) {
      var x = document.getElementById(prefix + "_s");
      x.play();
    } else {
      alert("wrong selection");
    }
  } else {
    alert("no");
  }
}

and in onclick, call image_select_animal("pig") or equivalent for the other animals.

answered Sep 30, 2017 at 20:35

Samuel Caillerie

8,2652 gold badges29 silver badges34 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

afgboy Over a year ago

hi @Samuel Caillerie Thank you for taking your time and to answer my question. i tried what you wrote but unfortunately does not work :( .

afgboy Over a year ago

i really like your solution but unfortunately does not work

Samuel Caillerie Over a year ago

I have just refactor your JS script, but it seems that your snippet is missing some part : image_select and others are never called...

Daniel Apt · Accepted Answer · 2017-09-30 21:21:49Z

0

function image_select(name) {
    var el = document.getElementById(name); // create this variable, as you use this element twice. Less DOM lookups.
    var pic = el.value; // get the value of the `el` variable
    var text = document.getElementById(name + '_t').value;

    if (!el.checked) { // if not check
        alert('no');
        return; // return ends the function immediately, meaning the following lines are not read
    }

    if (pic != text) {
        alert('wrong selection');
        return; // return ends the function immediately, meaning the following lines are not read
    }

    document.getElementById(name + '_s').play(); // no need to store this in variable x, as you only use this element once.
}

edited Sep 30, 2017 at 21:21

answered Sep 30, 2017 at 21:00

Daniel Apt

2,7181 gold badge25 silver badges36 bronze badges

16 Comments

afgboy Over a year ago

can you explain me a bit more plz

Daniel Apt Over a year ago

@afgboy - I believe to have fixed it. Also, I added more comments. Does this help?

afgboy Over a year ago

for the function image_select(name) do i need to change (name) with something or should i leave it like this ?

Daniel Apt Over a year ago

No, you keep it as is: name is an argument. It means that when you call image_select() you provide a value inside the (). E.g. you would call image_select('cow') or image_select('pig'). That way you have only one function, and keep re-using it. In general, if you are writing the same code across different functions, you should put the repeating code into its separate function.

afgboy Over a year ago

people like you make this world beautiful

|

Collectives™ on Stack Overflow

how to make this javascript code shorter?

2 Answers 2

3 Comments

16 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

16 Comments

Your Answer

Sign up or log in

Post as a guest

Related