How to improve this jQuery code?

Asked

Viewed 268 times

9

I’m learning jQuery and I don’t always know exactly how to develop the code more cleanly, semantically, I’ll end up learning. How could I improve this code?

$("#click_apoio").on('click', function( evento ){

    evento.preventDefault();

    if( $("#institucional").hasClass('desativado') ){

        $("#institucional").removeClass('desativado');
        $("#institucional").addClass('ativado');
        $("#parceiros").removeClass('ativado');
        $("#parceiros").addClass('desativado');

    }

});
javascript jquery
  • 4

    You can also think of another structure... you are using two classes: enabled and disabled... if you used only one class, you would use instead of removeClass and addClass.. the toggleClass method... and would adapt the css to suit this structure...

  • 1

    I agree with what Peter said !!!

  • 2

    Dude, sometimes making code cleaner doesn’t mean having less line amount, it’s making it legible. Your code is good and to be understood. Another important factor is to check if you have any repeated code, to separate into functions

  • 1

    @Ericosouza is exactly right! As I commented below, there is nothing "unprofessional" in this code, it is good, can perfectly understand what you are doing!

  • Thank you all for finding the issue interesting enough for a short debate.

  • A question... is there the need for jQuery in its function? Was that question only at the level of learning? Because in my view, I could very well achieve the same goal only with Avascript

  • So @Marcelobonifazio I am part of the group of people who learn jQuery without really knowing Javascript. Javascript study but not to the point of prioritizing it over jQuery. :)

  • 1

    Yes, it is valid, but keep in mind that using jQuery affects the performance of the page, even if little, although with jQuery you have access to many useful things that only with pure Avascript becomes more complicated to do

  • This question is being discussed in the target: http://meta.pt.stackoverflow.com/questions/4210/eu-sou-f%C3%A3-but-unfortunately-this-tool-inhibits-participates%C3%A7%C3%B5es-porqu%C3%AA-tem-qu

  • As it is I did not vote to close, alias withdrew the vote. Although I still do not like most of the questions.

Show 5 more comments

4 answers

10


Like good javascript practices, it is interesting to create variables of each element that will be manipulated, to avoid too many accesses to DOM trees. Look at these modifications I made to the code:

$(function() {

  $("#click_apoio").on('click', function(e) {

    e.preventDefault();
    var $institucional = $("#institucional"),
      $parceiros = $("#parceiros");

    if($institucional.hasClass('desativado')) {

        $institucional.removeClass('desativado')
          .addClass('ativado');

        $parceiros.removeClass('ativado')
          .addClass('desativado');    
    }    

  }); 

});

About the modifications:

  • I added an anonymous jQuery function ($(function){...});) to prevent the script from being loaded before the jQuery object exists on the page;
  • I created variables for each element to be manipulated, avoiding unnecessary access to the DOM tree;
  • I nested functions in each element, to organize the code and make it more readable (Ex: $institucional.removeClass('desativado').addClass('ativado'););
  • 1

    Cool this huh @Henrique !!!

  • This code is giving me a Reload on the page huh ....

  • Take a look at the console of the page. Probably I must have missed when typing the code.

  • I made a very crude example just to show that the function can be used toggleClass also. https://jsfiddle.net/rm5d55bf/1/

7

Your code is best used in another logic, for example, if there are only two states ativado and desativado, so you can omit the "disabled" leaving the default element with this behavior, and change only if there is a class="ativado".

Example:

$("#click_apoio").on('click', function(e){
    e.preventDefault();
    $("#institucional, #parceiros").toogleClass('ativado');
});

4

$("#click_apoio").on('click', function( evento ){
    evento.preventDefault();
    if($("#institucional").hasClass('desativado')){
        $("#institucional").removeClass('desativado').addClass('ativado');
        $("#parceiros").removeClass('ativado').addClass('desativado');
    }
});

Good gave to decrease a little of line.

  • 1

    I had tried to concatenate the instructions but it had not worked. Thank you

  • But you just removed the lines, didn’t improve the code at all,

  • rsrsrsrs .... gives your option @Gerep !!!!

  • @Marcosvinicius That’s right, there’s nothing "not professional" in this code, jQuery is like this!

  • Thank you @Gerep for letting me know that I’m not that new!!!! rsrsrs :)

  • There is nothing else to do in the code @Gerep, it is right :) The only thing he can do if you are going to reference this ID a lot in your code is to create variables to make it more "intelligible"

  • 1

    @Danilo, yes, I understood and no doubt your suggestion is very valid but I do not understand why you think that this is not "intelligible" enough, the code this super clear and direct, in my opinion, I see no need to change :)

  • 1

    For example, I think it was useful for @Marcosvinicius, because he wasn’t able to concatenate.

Show 3 more comments

4

I don’t believe you have any problem with your code, it’s not less professional because it doesn’t look complex or because it has several lines.

Often cute algorithms are more error-prone and harder to debug, and are also harder to implement and understand if another programmer needs it.

Always focus on developing simple and clear algorithms, even if it makes it verbose (there are other factors to consider).

Summarizing and using the unix philosophy:

Rule of Clarity: Clarity is Better than cleverness.

  • Agree with you @Gerep

  • @Marcosvinicius that good =) Then take a look at the link I sent in the reply, is very interesting and for me is a reference.

  • I agree that what matters in the end is clean, easy-to-understand code. I also agree that there are always other factors to consider and, when it comes to web, the performance of the code is also very important, because the accesses to the sites is increasingly being made by mobile devices, so the importance of seeking the best performance.

  • @Henriqueschreiner is quite right. There is also a rule in Unix philosophy that touches a little on this subject: Rule 2. Measure. Don't tune for speed until you've measured, and even then don't unless one part of the code overwhelms the rest.

  • 1

    Like that expression: Shoot ants with bazooka!

Browser other questions tagged javascript jquery

You are not signed in. Login or sign up in order to post.