-1

I just learned about functions and I'm trying to test a simple function:

<button id="testbutton">test</button>
<script type="text/javascript">
  var testTime = ""

  function alertTime(inputTime) {
    inputTime = Date.now();
    alert(inputTime);
  }
  document.getElementById("testbutton").onclick = alertTime(testTime);
</script>

Why is the alert showing up as soon as I load the page, and not when I click the button?

Ivar
  • 6,138
  • 12
  • 49
  • 61
Ben
  • 311
  • 4
  • 10
  • Because you're calling it. – Teemu Jan 15 '20 at 12:36
  • @Teemu, Everyone knows that function is called when it has been called, but can you tell him where has called that function – Ahmed Ali Jan 15 '20 at 12:37
  • Does this answer your question? [addEventListener calls the function without me even asking it to](https://stackoverflow.com/questions/16310423/addeventlistener-calls-the-function-without-me-even-asking-it-to) – Ivar Jan 15 '20 at 12:38
  • 1
    The alertTime function will be evaluated before being assigned to the onclick event handler. Because it will be evaluated, the alert will be shown. – Henridv Jan 15 '20 at 12:39

5 Answers5

2

Function are called/invoked immediately with (). You have to call/invoke the function inside of an anonymous function.

Change

document.getElementById("testbutton").onclick = alertTime(testTime);

To

document.getElementById("testbutton").addEventListener('click', function(){ alertTime(testTime) });
Mamun
  • 66,969
  • 9
  • 47
  • 59
2

Why is the alert showing up as soon as I load the page

The JS interpreter finds <script> tags parsing the page and executes what is enclosed inside. Inside you call alertTime() (function name + () results in a function call) with argument testTime. Then execution goes into alertTime() where browser environment's function alert() gets called and you finally see the message.

curveball
  • 4,320
  • 15
  • 39
  • 49
0

Actually, you are not binding a function but a result of execution of a function that is why you have this side effect.

Now you can correct using a callback like this

<button id="testbutton">test</button>

        <script type="text/javascript">
            var testTime = ""

            function alertTime(inputTime) {
                inputTime = Date.now();
                alert(inputTime);
            }

            document.getElementById("testbutton").addEventListener('click', function(){ 
  alertTime(testTime);
});

        </script>
Patrissol Kenfack
  • 764
  • 1
  • 8
  • 22
0

In JavaScript functions are called using the follow syntax:

functionName()

When using an event handler, each event will call the functions assigned as event handlers. You do not want to call the function but instead set the function value as the event handler, like this:

function alertTime(inputTime) {
    inputTime = Date.now();
    alert(inputTime);
}

element.onclick = alertTime;

This is why the alert appears immediately: you are calling your function, instead of assigning it to the onclick event handler.

You will notice this doesn't pass along the testTime variable. While you could bind the current value of testTime, "", for every single time the event handler is called, like this:

element.onclick = alertTime.bind(this, testTime)

...it is most likely not what you want, as the event handler will always be called with "" even if testTime changes. Instead, try using testTime inside your function directly.

metarmask
  • 1,747
  • 1
  • 16
  • 20
  • Why to mix `this` to it, it is never used in the OP's code. The first example here is fine, if you'd just removed the argument from the argument list of the function definition, `let = ` is shorter to write than `inputTime`. – Teemu Jan 15 '20 at 12:54
  • @Teemu Because they were trying to pass along an argument to the event handler, which could have some use cases. In order to use `bind` I had to pass an argument for the `this` to bind. What better value than `this`, which matches what the function is called with by default? – metarmask Jan 15 '20 at 13:01
  • Yes, I can undestand that, but OP's example conflicts with itself, as well as your first example. It's not usefull to pass an argument, and then immediately override it without even using the value of the passed argument. – Teemu Jan 15 '20 at 13:03
  • @Teemu I agree, and in addition I do mention in my answer that `bind`ing the variable is not useful for changing the variable in the function. Maybe it would have been a better answer if I had commented on the content of the function as well. – metarmask Jan 15 '20 at 13:30
0

It's a good idea to keep the logic of a function contained within itself, unless your use case needs it.

That way you won't need to pass any arguments to the alertTime function when you set it as the callback for the click event listener and it won't be evaluated.

In your case bind would fix the problem.

document.getElementById("testbutton").onclick = alertTime.bind(testTime);

but you can make things a bit simpler and not have to use bind like so:

const button = document.getElementById("testbutton");

const alertTime = () => {
  const time = Date.now();
  alert(time);
};

button.addEventListener('click', alertTime);
<button id="testbutton">test</button>

I also recommend reading about arrow functions since you mentioned that you're learning about JS functions

volt
  • 963
  • 8
  • 17