0

Here's the html code.

<script type="text/javascript">
function func() {

    if (document.getElementById("heart").src == "heart_empty.png") 
    {
        document.getElementById("heart").src = "heart_filled.png";
    }
    else if(document.getElementById("heart").src == "heart_filled.png")
    {
        document.getElementById("heart").src = "heart_empty.png";
    }
}
</script>
<img id="heart" src='heart_empty.png' onclick="func()" />

Javascript function func() is not working.

8
  • 1
    What exact error message do you get in the console? Commented Apr 27, 2017 at 21:11
  • 4
    second if uses = but should use == Commented Apr 27, 2017 at 21:12
  • else if(document.getElementById("heart").src = "heart_filled.png") here you're doing assignment and not test Commented Apr 27, 2017 at 21:12
  • Image src does not change on click. Commented Apr 27, 2017 at 21:12
  • changed '=' to '==', still nothing Commented Apr 27, 2017 at 21:13

4 Answers 4

3

Your second else if has a single =. It should be a comparison, so it should use == like the first.

Always try to use the strict comparison operator === to ensure that you're dealing with the same types. If the types are different, you should convert it before comparison.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
        document.getElementById("heart").src = "heart_filled.png";
    } else if(document.getElementById("heart").src == "heart_filled.png") {
        document.getElementById("heart").src = "heart_empty.png";
    }
}

In addition, since it looks like there is only ever two states, you don't have to have a second else if. Just skip to the else.

function func() {
    var heart = document.getElementById("heart");

    if (heart.src == "heart_empty.png") {
        heart.src = "heart_filled.png";
    } else {
        heart.src = "heart_empty.png";
    }
}
Sign up to request clarification or add additional context in comments.

5 Comments

Even better, don't ever use even the == (comparison with conversion) operator as it can return false positives/negatives. Use the === (strict equality) for all comparisons.
@ScottMarcus Completely agreed, updated answer. Feel free to update if you have any other additions :)
I would just suggest that you always use ===. Then you will have to take responsibility for type conversion, which is actually a good thing as it eliminates the possibility of bugs.
how does this help answering the question tho?
@CharlieNg It's more a case of explaining what the difference is between = and a comparison operator. It could've been a typo in OPs code, or they don't know the difference. The question explains the difference
0

Can you use background image instead? So have a div and depending on the class you have different image as the background.

HTML

<div class="heartImg filled"></div>

CSS

.filled {
    background-image: url(filled.jpg);
}
.empty {
    background-image: url(empty.jpg);
}

Jquery

$('.heartImg').click(function(){
    $(this).toggleClass('filled empty');
}

1 Comment

This would require the html to start off with either class.
0

Hello

Try to use the operator "?" for this instance

Your current code looks like:

function func() {
  if (document.getElementById("heart").src == "heart_empty.png") 
   {
  document.getElementById("heart").src = "heart_filled.png";
  }
  else if(document.getElementById("heart").src == "heart_filled.png")
  {
      document.getElementById("heart").src = "heart_empty.png";
  }
}

Instead, use the "?" operator to compact your function into one line:

Also, instead of getting the element each time, add a script at the bottom of the page that looks like this:

var src = document.getElementById("heart").src;

That will not only decrease lag but also will increase readability of your code

The new script will use the fact that the "?" operator can combine an "if" statement into a one line declaration. To recap, the operator does this:

variable = condition ? /*true*/ : /*false*/

Put your (true) code in the "true" bit and same with the false bit If the condition is true, then it will set the "variable" to the code written in the "true" bit, otherwise, it will set it to the code written in the "false" bit.

So now with this newfound knowledge, we can update our code!!!

NEW CODE:

function func() {
  document.getElementById("heart").src =
  document.getElementById("heart").src === "heart_empty.png" ?
  "heart_filled.png" : "heart_empty.png";
}

This can be further compacted by using the notation stated above where "src" variable = document.getElementById("heart").src, now it looks like:

function func() {
  src = src === "heat_empty.png" ? "heart_filled.png" : "heart_empty.png";
}

Much Better! Now your code is neat, compact and flawless!

Comments

0

The second of your else if statements has one = rather than two. It needs to be a comparison, so use ==.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
    document.getElementById("heart").src = "heart_filled.png";
    } else if(document.getElementById("heart").src == "heart_filled.png") {
    document.getElementById("heart").src = "heart_empty.png";
    }
}

Also, because you only have two statements, you can simply use else instead of else if.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
    document.getElementById("heart").src = "heart_filled.png";
    } else (document.getElementById("heart").src == "heart_filled.png") {
    document.getElementById("heart").src = "heart_empty.png";
    }
}

1 Comment

Your answer is not wrong but actually don't contribute anything new, please check other answers first!

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.