1
\$\begingroup\$

A friend helped me put the following code together, which is made up of a series of repetitive loadContent() functions. I've just shown the first two functions but they're all the same.

The code works fine but I'm wondering if anyone could give me a tip as to how I could reduce the bloat in this code and condense the functions somehow. I read somewhere that repetitive code usually means that the code can be simplified or shortened somehow. I'm no expert at jQuery or coding so I'm not sure where to start. I'm assuming that the code could be written a lot better but I might be wrong.

function loadContent1()
 {
 $('#content1').empty();
 if(typeof(counterArray[1]) != 'undefined' && counterArray[1] != null)clearTimeout(counterArray[1]);
 jQuery.ajax({
 type: "GET",
 url: "//cgi-bin/mggsn_ssr21.cgi",
 beforeSend:function(){
 jQuery('#content1').append('<img class="loading" src="img/loading.gif" alt="Loading..." />');
 jQuery('#refresh1text').html("<p>Loading...</p>");
 }
 })
 .done(function(data){
 if(typeof(counterArray[1]) != 'undefined' && counterArray[1] != null)clearTimeout(counterArray[1]);
 $('#content1').empty();
 $('#content1').html(data);
 //alert(data.slice(0.100));
 countDownTimeArray[1]=countDownIntervalArray[1];
 countDown(1);
 })
 .fail(function(){
 $('#content1').empty();
 jQuery('#content1').html('<h4>failed to load content</h4>' + this.url);
 });
 }
 function loadContent2()
 {
 $('#content2').empty();
 if(typeof(counterArray[2]) != 'undefined' && counterArray[2] != null)clearTimeout(counterArray[2]);
 jQuery.ajax({
 type: "GET",
 url: "/cgi-bin/mtest2.cgi",
 beforeSend:function(){
 jQuery('#content2').append('<img class="loading" src="img/loading.gif" alt="Loading..." />');
 jQuery('#refresh2text').html("<p>Loading...</p>");
 }
 })
 .done(function(data){
 if(typeof(counterArray[2]) != 'undefined' && counterArray[2] != null)clearTimeout(counterArray[2]);
 $('#content2').empty();
 $('#content2').html(data);
 //alert(data.slice(0.100));
 countDownTimeArray[1]=countDownIntervalArray[1];
 countDown(2);
 })
 .fail(function(){
 $('#content2').empty();
 jQuery('#content2').html('<h4>failed to load content</h4>' + this.url);
 });
 }
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 1, 2014 at 5:01
\$\endgroup\$
0

2 Answers 2

1
\$\begingroup\$

You could simplify like so:

For the HTML, you could embed specifics as data-* attributes, which you can fetch in JS later on.

<div class="content" data-source="//cgi-bin/mggsn_ssr21.cgi" data-refreshtext="#refresh1text'>
 ...
</div>
<div class="content" data-source="/cgi-bin/mtest2.cgi" data-refreshtext="#refresh2text'>
 ...
</div>

As for the loading gif, I'm not a fan of <img>. I'm more into making the loading gif a background that's coupled with a class, so that it's easy to add using addClass and remove using removeClass.

.content.loading{
 background-image: url(img/loading.gif);
 background-repeat: no-repeat;
 background-position: center center;
 min-height: 100px;
}

As for the function, we grab the data-* attributes, and execute the ajax.

function loadContent(selector){
 var element = $(selector);
 var url = element.data('source');
 var refreshSelector = element.data('refreshtext');
 $.ajax({
 type: "GET",
 url: url,
 beforeSend: function () {
 // Empty the element and set it to loading
 element.empty().addClass('loading');
 $(refreshSelector).html("<p>Loading...</p>");
 }
 }).done(function(data){
 element.html(data);
 ...
 }).fail(function(){
 element.html('<h4>failed to load content</h4>' + this.url);
 ...
 }).complete(function(){
 // for all cases, fail or success, remove the gif background
 element.removeClass('loading');
 });
}
// use like:
loadContent('.content');

You may supply any selector as your parameter here, like an ID. Just make sure it has the right data-* attributes to go with it.

answered Jun 1, 2014 at 5:52
\$\endgroup\$
0
2
\$\begingroup\$

Joseph cleaned up the code a lot so let me point out a few things that you'll be able to use in your future code.

  • Consistent formatting greatly improves code readability and maintenance. There are many out there that are good starts, and I'm a fan of Google's language styles for the most part.

  • Since you're already depending on the definition of $, it's odd to mix it with jQuery in the same code. Pick one and stick with it.

  • To check that a value is neither null or undefined you can simply compare it to null using ==/!=. When you know the value cannot be some other falsy value (e.g. false, 0, or ''), you can simply use ! or the boolean context.

    if (counterArray[1]) { ... }
    
  • DOM queries can be expensive on large pages, so it pays to save the results of a query if you're going to use it multiple times, and it cleans up the code. I like prefixing the name of a variable holding the results of a selector with a $ to make it clear that it's not a raw DOM element.

    var $content = $('#content1');
    
  • Most jQuery methods return the selector itself, allowing you to chain multiple calls together.

    $content.empty().html(data);
    
  • html completely replaces the contents of the element so there's no reason to call empty on it first. DOM manipulation causes a reflow; combine your manipulations when possible.

answered Jun 1, 2014 at 7:04
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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.