1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

Is this way of writing code as bad as i think it is

Discussion in 'Programming' started by mumfry, Oct 24, 2014.

0
  1. #1
    Hello guys..

    This is what the code looks like..

    $('body').on('click','.preview_images', function(){
  $('#image_container').hide();
});

$('body').on('click','.popwrap', function(){
    $('#cancel').hide();
});

$('body').on('click','.cancel_pop', function(){
    $('#main_bttn').show();
});

$('body').on('click','.images', function(e){
        e.preventDefault();
$('.loader').show();
});
    Code (JavaScript):
    So it would be a full js file of code written like above basically, different event listeners(like click, submit, mouseover and so on) set to different selectors that then perform actions like hide and show other divs and what not.

    But when i look at the way the "Pros" write their code. I never tend to see a bunch of event listeners like the way i do it. They tend to create lots of objects and methods within those objects.

    The way i do it is very basic but it works, i like to follow the advise of KISS(keep it simple stupid). So my question is, is there anything wrong with writing the code like i tend to do or is it ok. Even, is there a better way of doing this. Would you do it differently, If so what would you do differently

    Would love to hear from the coders on the forum in regards to this

    Thanks
     
    mumfry, Oct 24, 2014 IP
  2. Anveto

    Anveto Well-Known Member

    Messages:
    697
    Likes Received:
    40
    Best Answers:
    19
    Trophy Points:
    195
    #2
    I think it is fine to write like this but you should work on maybe creating your own functions and using an array to store all these functions. This is a good way to reduce code. Later you can move into objects and whatnot once you get the hang of arrays and functions.

    If you get less errors doing this I would keep doing this on public projects but practice on "better" ways on private test projects. Basically you will create sites with better performance if you learn to make things simpler.
     
    Anveto, Oct 24, 2014 IP
  3. seductiveapps.com

    seductiveapps.com Active Member

    Messages:
    200
    Likes Received:
    6
    Best Answers:
    0
    Trophy Points:
    60
    #3
    It's not bad per se.

    I would definately skip putting event handlers in an array, i'd go straight for an object for the entire app like this :
    
myFrameworkName = {};

myFrameworkName.myApp = {
    globals : {
    },
    settings : {
    },
  
    startApp : function () {
        jQuery('#logo').click(myFrameworkName.myApp.ui.eventHandlers['#logo'].click);
            // eventually you could write a function that semi-recursively loops through myFrameworkName.myApp.ui.eventHandlers and ties all the functions there via jQuery without having to update this list here in this function each time you update ...ui.eventHandlers.
    },
  
    ui : {
        eventHandlers : {
            '#logo' : {
                click : function (evt) { // evt = event data
                }
            }
        }
    }
};

/*
called in HTML like :

<body onload="myFrameworkName.myApp.startApp()">
...
</body>
*/
    Code (markup):
     
    seductiveapps.com, Oct 25, 2014 IP
    sarahk likes this.
  4. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,623
    Likes Received:
    725
    Best Answers:
    152
    Trophy Points:
    470
    #4
    How is that better? I'm asking, because I'm really not a fan of dispersing javascript inline in the HTML. From a maintenance standpoint, that looks like a mess.
     
    PoPSiCLe, Oct 25, 2014 IP
  5. Anveto

    Anveto Well-Known Member

    Messages:
    697
    Likes Received:
    40
    Best Answers:
    19
    Trophy Points:
    195
    #5
    Just because he wrote it inline doesn't mean you have to use it inline. You could put the js in a .js file and you probably should.
     
    Anveto, Oct 25, 2014 IP
  6. deli6z

    deli6z Greenhorn

    Messages:
    7
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    21
    #6
    what I would change here is the following:
    var selector = $('body');
    so that you don't need to run jQuery to select body dom element each time you deal with it.
     
    deli6z, Oct 26, 2014 IP
  7. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #7
    My problem with it would be that as of CSS3 being real world deployable, there's no reason to be wasting javaScript on showing/hiding things. If it's something like one of those goofy "let's piss on mobile by hiding the menu to make mobile easier" things, just use :target. If it's an accordion menu just abuse a checkbox or radio button with :checked>element to show/hide. (hiding the input element and using LABEL as the selector)

    Though for the most part all this type of crap just makes sites harder to use or encourages slapping too much navigation on a single page -- there's a reason I call it "JS for nothing"; but as I keep saying 99% of what people do using jQuery and other JS frameworks falls into one of three categories:

    1) Stuff that would be less code without the framework (NOT counting the size of the framework against the dev written code!)

    2) Stuff that is CSS' job

    3) "Gee ain't it neat' bull that has no business on a website in the first blasted place!
     
    deathshadow, Nov 5, 2014 IP
  8. mumfry

    mumfry Active Member

    Messages:
    118
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    51
    #8
    Thanks a lot guys...
    I appreciate yall taking the time to respond.
    Also its a bit reassuring knowing that am not doing the worst thing by writing my code like this.
    Deli6z thanks for the advise to use this
    var selector = $('body');
    Code (JavaScript):
    Instead of calling body each time, this was something i did implement earlier on in the project.
    Thanks to seductiveapps.com for that bit off code also... some very advanced ideas there

    Hopefully if i have issues in the future i can message you guys for advise.:)
     
    mumfry, Nov 8, 2014 IP
  9. subdivisions

    subdivisions Well-Known Member

    Messages:
    1,021
    Likes Received:
    40
    Best Answers:
    1
    Trophy Points:
    145
    #9
    Yeah, as a rule you should never call the same $('.something') more than once, save it to a variable and use the variable. Each query takes a few milliseconds from your page speed, no sense in wasting it selecting the same thing over and over.
    var test = $('.test');
test.foo();
test.bar();
    Code (markup):
    Also, is there a reason you're binding your listeners to the body and not the elements themselves? It's more succinct to just do $('.preview_images').click() but you don't have much choice if the elements are dynamically generated.
     
    subdivisions, Nov 11, 2014 IP
  10. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,623
    Likes Received:
    725
    Best Answers:
    152
    Trophy Points:
    470
    #10
    Uhm... sure you do? Just do $('.preview_images').on('click',function() {}) it will cater for dynamically created content as well.
     
    PoPSiCLe, Nov 12, 2014 IP
  11. seductiveapps.com

    seductiveapps.com Active Member

    Messages:
    200
    Likes Received:
    6
    Best Answers:
    0
    Trophy Points:
    60
    #11
    'more than once'.. look dude, a call like that isn't going to take a full millisecond. unless you're running it in a long loop, you might aswell call it directly every time.
     
    seductiveapps.com, Nov 12, 2014 IP