Problem
I’m working on a custom (self-hosted) ads solution that allows the users to create ad units (by uploading the banner image and target URL) and get the ad display code. A JSON response is returned (by the endpoint) for the requested ad. As of now, here is what I do to get and display the ad on the page where ad code is added:
JSON output at the endpoint:
{
"success": true,
"ad_url": "https://adlandingpage.com",
"ad_banner": "http://localhost/ads/complete/uploads/banners/add-banner.gif"
}
Content of rw-ads.js:
var getAds = function(ad_id, callback) {
url = 'http://localhost/ads/complete/index.php/ads/adjson/'+ad_id+'/';
var xhr = new XMLHttpRequest();
xhr.open('get', url, true);
xhr.responseType = 'json';
xhr.onload = function() {
var status = xhr.status;
if(status == 200) {
callback(null, xhr.response);
} else {
callback(status);
}
};
xhr.send();
};
Ad code to be added to a page:
<div id="ad1"></div>
<script src="http://myadsplatform.tld/assets/js/rw-ads.js"></script>
<script>
var ad_id = 1;
getAds(ad_id, function(err, data) {
if(data.success) {
$('#ad1').html('<a href="'+data.ad_url+'"><img style="max-width: 100%;" src="'+data.ad_banner+'"></a>');
}
});
</script>
I need suggestions from experts. Am I doing this correctly or this can be improved? Thank you in advance for your help.
Solution
A few suggestions:
- A function declaration is a bit simpler and more robust regarding hoisting compared to a function expression which assigns an anonymous function to a variable.
- Supplying
null
as the first callback parameter on success is a bit quirky, either always supply the HTTP status or fail silently without calling the callback. If needed you can always introduce an error callback which might be called onxhr.onerror
, too. - The
async
parameter of xhr.open defaults totrue
and can be removed - Your
getAds
function could be split into a genericgetJSON
function and agetAd
function which supplies the specific ad URL to the former. - Your current
getAds
callback requires knowledge about the specific JSON returned by your ad server. This logic should be part of your “rw-ads.js” library and e.g. become part of the newgetAd
function.
Exemplary code:
function getJSON(url, success) {
var xhr = new XMLHttpRequest();
xhr.open('get', url);
xhr.responseType = 'json';
xhr.onload = function() {
if (xhr.status === 200) {
success(xhr.response);
}
};
xhr.send();
};
function getAd(id, success) {
getJSON('http://localhost/ads/complete/index.php/ads/adjson/' + id, function(data) {
if (data.success) {
success('<a href="' + data.ad_url + '"><img style="max-width: 100%;" src="' + data.ad_banner + '"></a>');
}
});
}
getAd(1, function(html) {
document.getElementById('ad1').innerHTML = html;
}
General considerations:
Since you don’t rely on an external ad network but a self-hosted solution, you might want to inject your ads into your HTML document on the server side, without the asynchronous client side ajax request.