A reusable AJAX polling function

Question 1

I have written a small JavaScript function which performs an AJAX request every x seconds.

I was hoping some of you could give it a look over and give me any pointers on redundant code or bad practices.

The idea of the function is to:

Allow for polling to be initiated
Once initiated, poll once every x seconds and perform an AJAX request.
Stop polling once disablePolling() is executed.
Be able to be reused anywhere.

requester.js

function Request() {
this.poll = false;
this.activatePoll = function () {
 this.poll = true;
 this.runPoll();
};
this.disablePoll = function () {
 this.poll = false;
};
this.runPoll = function () {
 var self = this;
 var poll = setTimeout(function () {
 $.ajax({
 url: './request.php',
 success: function (response) {
 console.log(response);
 },
 dataType: "json",
 complete: function () {
 if (self.poll == false) {
 clearTimeout(poll);
 } else {
 self.runPoll();
 }
 }
 })
 }, 1000);
};
}

Usage:

index.html

<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8">
 <title>Requester</title>
</head>
<body>
<h1>Requester</h1>
<div id="requester">
 <input type="button" onclick="$request.test();" value="test">
 <input type="button" onclick="$request.activatePoll();" value="poll">
 <input type="button" onclick="$request.disablePoll();" value="turn off poll">
</div>
</body>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script>
<script src="js/request.js"></script>
<script>
 $(document).ready(function () {
 $request = new Request();
 });
</script>
</html>

request.php

<?php
echo json_encode($time());
?>

It should also be mentioned that theAajax request will need to also POST some parameters.

Question 2

Your code is simply a GET request, so the shorthand getJSON can be used for this.
Instead of success and complete, use the more standard Promise method then.
Instead of a recursive setTimeout, use setInterval instead. This avoids your code from rescheduling a callback on each iteration. You can always clear interval timers with clearInterval.
When using setInterval, clearing the timer will suffice for stopping the polling. You don't need the boolean flag to tell you to stop.
When using setInterval, you don't need an extra function for a recursive call anymore.
You also seem to use OOP. It is normally advised to put methods on the prototype and not the instance so that methods are shared per instance and not created per instance.
It is usually not advised to use inline scripts. Use addEventListener to bind your click handlers.
Booleans are usually prefixed with an is, has or are depending on the grammar. This is to not mistake a property as some other non-boolean value.
Put the url and interval in a variable that acts as a constant. That way, you can easily configure as well as spot the values and not dig through code.
If ES6 is available for you, self = this can be taken care of by using arrow functions.

In the end, your code could be as simple as

function Request() {
 this.pollTimer = null;
 this.interval = 1000;
 this.url = './request.php';
}
Request.prototype.disablePoll = function () {
 clearInterval(this.pollTimer); 
 this.pollTimer = null;
};
Request.prototype.activatePoll = function () {
 this.pollTimer = setInterval(() => {
 $.getJSON(this.url).then(response => console.log(response))
 }, this.interval);
};

Question 3

The only issue with setInterval and ajax is the order of console.log is not guaranteed to be sequential.

Question 4

thank you Joseph. 1 question though. By using setInterval() and then() instead of setTimeout(). Will this still run the next request only after the previous request has completed?

Question 5

it should also be mentioned that the ajax request will need to also POST some parameters.

Question 6

@BradM But then, no one leaves console.log in production code. :D

Question 7

@ayala Your question made no mention doing a sequential poll. Your question was "poll once every x seconds and perform an AJAX request" and "function which performs an AJAX request every x seconds."

Question 8

I prefer to do object definition in an IIFE, unless you're willing to use Babel or some other transpiler. I've included comments detailing other thoughts on certain pieces. I also agree with Joseph's recommendations too so I did my best to try not to repeat much (if any) of them. Here is what your Request definition would look like:

var Request = (function() {
 function Request(url, interval) {
 this.poll = false;
 this.interval = null;
 this.url = url;
 this.interval = interval || 1000;
 }
 var proto = Request.prototype;
 // other potentially better names could be 'startPolling', 'start' or just 
 // 'poll'
 proto.activatePoll = function() {
 this.poll = true;
 this.runPoll();
 };
 // other potentially better names could be `stopPolling`, `end` or `stop`, 
 // `endPolling`
 proto.disablePoll = function() {
 this.poll = false;
 };
 // other potentially better names could be `run`, `executed`
 // also these feels "private" so I'd prefix it with an _ like _runPoll just
 // to signify "you probably don't need to use this"
 proto.runPoll = function() {
 // I prefer _this over self because `this` is a known value in JS and self
 // is just a name. Admittedly _this is a name too but it's still 
 // distinguishable
 // also, group var declarations. No reason to see `var` scattered in a 
 // function. vars are function level declarations anyways so regardless of
 // where you define them they're accessible in function scope -- putting 
 // them first and gruoped is a clear way to express this
 var _this = this,
 poll;
 if (!this.poll) {
 // If this gets called when we're not polling, we don't need to create 
 // a timeout.
 return;
 }
 poll = setTimeout(function() {
 $.ajax({
 url: _this.url,
 success: function(response) {
 console.log(response);
 },
 dataType: "json",
 complete: function() {
 // we don't need to test here, because we're returning from runPoll
 // if we're not polling.
 _this.runPoll();
 }
 });
 }, _this.interval);
 };
 return Request;
})();

edit

I did alter your API contract though, you really should think about reusability when you're building "classes." In this case, you take a required first parameter which is the URL you wish to request. There is an optional second argument that sets the interval but will specify to 1000 if none is given. This gives you flexibility and easy reusability: new Request("/request.php") or new Request("/poll_less_often.php", 5000).

Question 9

Thank you izuriel for your answer and information. Very helpful. I have adjusted my code in the original post. I noticed you have used "var proto = Request.prototype;" rather than "Request.prototype.disablePoll = function () ...". What difference / improvement can this make? Or if easier, what is this called and ill do some googling.

Question 10

Well. It reduces keystrokes so it's much easier to type repeatedly, it's also what a mini fire would do (with smaller variables and less white space). But that's really preference. Do whichever makes you feel better.

Question 11

There are a lot of edits here so hopefully I successfully sifted through to what you're looking for.

Personally I liked your original approach better than your edit with prototypes. Both are perfectly fine but I like to put everything in the object rather than prototyping so as to know for sure I've got everything in one place. I use prototypes when I'm extending built-in date functionality or other libraries.

To make your code more portable I recommend treating it like a starting point; one that can be extended and modified as necessary. In my version below I give the option to override certain default values when constructing the object. You're not always going to be talking to 'request.php' every 2 seconds. This will be especially important if you minify your code. You don't want to have to sift through that code to adjust defaults. It's also good for if you want to have more than one poll running at once. Being able to initialize more than one object with different properties will allow that.

Here's what I would do...no jquery required.

function Request(url,seconds){
 var self = this;
 this.pollId = null;
 this.busy = false;
 this.data = null;
 //have a default that can be overridden with construction parameter
 this.url = "./request.php";
 if(typeof(url) !== 'undefined' && url !== null){
 this.url = url;
 }
 //have a default that can be overridden with construction parameter
 this.seconds = 2000;
 if(typeof(seconds) !== 'undefined' && seconds !== null){
 this.seconds = seconds;
 }
 this.startPoll = function(){
 this.pollId = setInterval(self.postAJAX,this.seconds);
 };
 this.stopPoll = function(){
 clearInterval(this.pollId);
 this.pollId = null;
 };
 //to be called on polling completion - can be overridden if needed
 this.success = function(response){
 console.log(response);
 };
 this.postAJAX = function(){
 //check if we're in the middle of a request
 if(this.busy){
 return;
 }
 //we're not so set the busy flag
 this.busy = true;
 var xmlhttp;
 if (window.XMLHttpRequest) {
 // code for IE7+, Firefox, Chrome, Opera, Safari
 xmlhttp = new XMLHttpRequest();
 } else {
 // code for IE6, IE5
 xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
 }
 var data = "json="+encodeURIComponent(this.data);
 //Send the proper header information along with the request
 xmlhttp.open("POST", this.url, true);
 xmlhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
 xmlhttp.setRequestHeader("Content-length", data.length);
 xmlhttp.setRequestHeader("Connection", "close");
 //state change handler
 xmlhttp.onreadystatechange = function(){
 if(xmlhttp.readyState == 4 && xmlhttp.status == 200) {
 //clear the busy flag
 self.busy = false;
 //log the response or whatever other action you want
 self.success(xmlhttp.responseText);
 }
 }
 xmlhttp.send(data);
 };
}

Question 12

You could use " url = url | './request.php' " for the parameter-check / assigning default-values. The same concerning "seconds". Or is there a special reason because the usage of if is more appropriate here?

Question 13

Since your going to manually implement the XHR handling jQuery would provide for you you may as well add sniffing and support for the newer and much easier to use fetch API, or provide a polyfill for it. Otherwise there is really no point to teach an outdated method for handling XHR requests that every library and it's predecessor has wrapped for ages now.

Joseph JosephJoseph 25.4k2 gold badges26 silver badges37 bronze badges · Answer 1 · 2016-03-03 18:08:15Z

Your code is simply a GET request, so the shorthand getJSON can be used for this.
Instead of success and complete, use the more standard Promise method then.
Instead of a recursive setTimeout, use setInterval instead. This avoids your code from rescheduling a callback on each iteration. You can always clear interval timers with clearInterval.
When using setInterval, clearing the timer will suffice for stopping the polling. You don't need the boolean flag to tell you to stop.
When using setInterval, you don't need an extra function for a recursive call anymore.
You also seem to use OOP. It is normally advised to put methods on the prototype and not the instance so that methods are shared per instance and not created per instance.
It is usually not advised to use inline scripts. Use addEventListener to bind your click handlers.
Booleans are usually prefixed with an is, has or are depending on the grammar. This is to not mistake a property as some other non-boolean value.
Put the url and interval in a variable that acts as a constant. That way, you can easily configure as well as spot the values and not dig through code.
If ES6 is available for you, self = this can be taken care of by using arrow functions.

In the end, your code could be as simple as

function Request() {
 this.pollTimer = null;
 this.interval = 1000;
 this.url = './request.php';
}
Request.prototype.disablePoll = function () {
 clearInterval(this.pollTimer); 
 this.pollTimer = null;
};
Request.prototype.activatePoll = function () {
 this.pollTimer = setInterval(() => {
 $.getJSON(this.url).then(response => console.log(response))
 }, this.interval);
};

The only issue with setInterval and ajax is the order of console.log is not guaranteed to be sequential.
thank you Joseph. 1 question though. By using setInterval() and then() instead of setTimeout(). Will this still run the next request only after the previous request has completed?
it should also be mentioned that the ajax request will need to also POST some parameters.
@BradM But then, no one leaves console.log in production code. :D
@ayala Your question made no mention doing a sequential poll. Your question was "poll once every x seconds and perform an AJAX request" and "function which performs an AJAX request every x seconds."

izuriel izurielizuriel 1514 bronze badges · Answer 2 · 2016-03-04 02:18:55Z

I prefer to do object definition in an IIFE, unless you're willing to use Babel or some other transpiler. I've included comments detailing other thoughts on certain pieces. I also agree with Joseph's recommendations too so I did my best to try not to repeat much (if any) of them. Here is what your Request definition would look like:

var Request = (function() {
 function Request(url, interval) {
 this.poll = false;
 this.interval = null;
 this.url = url;
 this.interval = interval || 1000;
 }
 var proto = Request.prototype;
 // other potentially better names could be 'startPolling', 'start' or just 
 // 'poll'
 proto.activatePoll = function() {
 this.poll = true;
 this.runPoll();
 };
 // other potentially better names could be `stopPolling`, `end` or `stop`, 
 // `endPolling`
 proto.disablePoll = function() {
 this.poll = false;
 };
 // other potentially better names could be `run`, `executed`
 // also these feels "private" so I'd prefix it with an _ like _runPoll just
 // to signify "you probably don't need to use this"
 proto.runPoll = function() {
 // I prefer _this over self because `this` is a known value in JS and self
 // is just a name. Admittedly _this is a name too but it's still 
 // distinguishable
 // also, group var declarations. No reason to see `var` scattered in a 
 // function. vars are function level declarations anyways so regardless of
 // where you define them they're accessible in function scope -- putting 
 // them first and gruoped is a clear way to express this
 var _this = this,
 poll;
 if (!this.poll) {
 // If this gets called when we're not polling, we don't need to create 
 // a timeout.
 return;
 }
 poll = setTimeout(function() {
 $.ajax({
 url: _this.url,
 success: function(response) {
 console.log(response);
 },
 dataType: "json",
 complete: function() {
 // we don't need to test here, because we're returning from runPoll
 // if we're not polling.
 _this.runPoll();
 }
 });
 }, _this.interval);
 };
 return Request;
})();

edit

I did alter your API contract though, you really should think about reusability when you're building "classes." In this case, you take a required first parameter which is the URL you wish to request. There is an optional second argument that sets the interval but will specify to 1000 if none is given. This gives you flexibility and easy reusability: new Request("/request.php") or new Request("/poll_less_often.php", 5000).

Thank you izuriel for your answer and information. Very helpful. I have adjusted my code in the original post. I noticed you have used "var proto = Request.prototype;" rather than "Request.prototype.disablePoll = function () ...". What difference / improvement can this make? Or if easier, what is this called and ill do some googling.
Well. It reduces keystrokes so it's much easier to type repeatedly, it's also what a mini fire would do (with smaller variables and less white space). But that's really preference. Do whichever makes you feel better.

OffTheBricks OffTheBricksOffTheBricks 4062 silver badges7 bronze badges · Answer 3 · 2016-03-04 23:54:05Z

There are a lot of edits here so hopefully I successfully sifted through to what you're looking for.

Personally I liked your original approach better than your edit with prototypes. Both are perfectly fine but I like to put everything in the object rather than prototyping so as to know for sure I've got everything in one place. I use prototypes when I'm extending built-in date functionality or other libraries.

To make your code more portable I recommend treating it like a starting point; one that can be extended and modified as necessary. In my version below I give the option to override certain default values when constructing the object. You're not always going to be talking to 'request.php' every 2 seconds. This will be especially important if you minify your code. You don't want to have to sift through that code to adjust defaults. It's also good for if you want to have more than one poll running at once. Being able to initialize more than one object with different properties will allow that.

Here's what I would do...no jquery required.

function Request(url,seconds){
 var self = this;
 this.pollId = null;
 this.busy = false;
 this.data = null;
 //have a default that can be overridden with construction parameter
 this.url = "./request.php";
 if(typeof(url) !== 'undefined' && url !== null){
 this.url = url;
 }
 //have a default that can be overridden with construction parameter
 this.seconds = 2000;
 if(typeof(seconds) !== 'undefined' && seconds !== null){
 this.seconds = seconds;
 }
 this.startPoll = function(){
 this.pollId = setInterval(self.postAJAX,this.seconds);
 };
 this.stopPoll = function(){
 clearInterval(this.pollId);
 this.pollId = null;
 };
 //to be called on polling completion - can be overridden if needed
 this.success = function(response){
 console.log(response);
 };
 this.postAJAX = function(){
 //check if we're in the middle of a request
 if(this.busy){
 return;
 }
 //we're not so set the busy flag
 this.busy = true;
 var xmlhttp;
 if (window.XMLHttpRequest) {
 // code for IE7+, Firefox, Chrome, Opera, Safari
 xmlhttp = new XMLHttpRequest();
 } else {
 // code for IE6, IE5
 xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
 }
 var data = "json="+encodeURIComponent(this.data);
 //Send the proper header information along with the request
 xmlhttp.open("POST", this.url, true);
 xmlhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
 xmlhttp.setRequestHeader("Content-length", data.length);
 xmlhttp.setRequestHeader("Connection", "close");
 //state change handler
 xmlhttp.onreadystatechange = function(){
 if(xmlhttp.readyState == 4 && xmlhttp.status == 200) {
 //clear the busy flag
 self.busy = false;
 //log the response or whatever other action you want
 self.success(xmlhttp.responseText);
 }
 }
 xmlhttp.send(data);
 };
}

You could use " url = url | './request.php' " for the parameter-check / assigning default-values. The same concerning "seconds". Or is there a special reason because the usage of if is more appropriate here?
Since your going to manually implement the XHR handling jQuery would provide for you you may as well add sniffing and support for the newer and much easier to use fetch API, or provide a polyfill for it. Otherwise there is really no point to teach an outdated method for handling XHR requests that every library and it's predecessor has wrapped for ages now.

Stack Exchange Network

A reusable AJAX polling function

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

A reusable AJAX polling function

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions