Simplification of javascript code [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

Want to improve this question? Because this question may lead to opinionated discussion, debate, and answers, it has been closed. You may edit the question if you feel you can improve it so that it requires answers that include facts and citations or a detailed explanation of the proposed solution. If edited, the question will be reviewed and might be reopened.

Closed 11 months ago.

Improve this question

Is it possible to compress this code below into few lines? There are similar _jq_overlay_x and _jq_overlay_y statements for 8 different events. I think that will be better to compress those lines which will be better for optimization.

Thanks in advance

  $(document).ready(function(){
        $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay1').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay1').fadeIn(500); 
        });
        $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay2').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay2').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay3').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay3').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay4').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay4').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay5').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay5').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay6').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay6').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay7').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay7').fadeIn(500); 
        });
            $('#_jq_overlay_x').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay8').fadeOut(500); 
        });

        $('#_jq_overlay_y').click( function(event) { 
            event.preventDefault(); 
            $('#_jq_overlay8').fadeIn(500); 
        });
 });

switz · Accepted Answer · 2012-03-12 10:13:04Z

1

Either use classes instead of ids or combine all of the similar ones into something like:

$("#object1, #object2, #object3, ...").click(function() { ... });

Then, just use one click and check if the object .is(':visible') to hide/show it.

answered Mar 12, 2012 at 10:13

switz

25.3k27 gold badges79 silver badges106 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Anthony Grist Over a year ago

This really doesn't sound like it answers the question at all.

KooiInc · Accepted Answer · 2012-03-12 11:03:24Z

1

I think it can be shortened to:

$('#_jq_overlay_x,#_jq_overlay_y').click( function(event) { 
   event.preventDefault();
   for (var i=1;i<9;(i=i+1)){
     $('#_jq_overlay'+i)[(/_y/i.test(this.id) ? 'fadeIn' : 'fadeOut')](500);
   }
});

See this jsfiddle mockup

edited Mar 12, 2012 at 11:03

answered Mar 12, 2012 at 10:21

KooiInc

124k32 gold badges145 silver badges181 bronze badges

4 Comments

Marko Over a year ago

this works in Firefox, Chrome, Safari, Opera, but it seems doesn`t work in IE7/8

KooiInc Over a year ago

The click handler function works. In the jsfiddle mockup, the boxes are not shown in IE7/8, because IE7/8 don't know the css rgb(a) property. I've adjusted the example, now it also works in all IE versions.

adeneo Over a year ago

+1 for a REALLY short solution and some cool scripting, but looking at the question I doubt the OP has any idea what is actually going on in that fiddle ?

KooiInc Over a year ago

thanks @adeneo. I've added a few comments to the jsfiddle example

adeneo · Accepted Answer · 2012-03-12 11:21:47Z

0

$(document).ready(function(){
    var elms=[];
    for (var i=1; i<=8;i++) {
        elms.push($('#_jq_overlay'+i)[0]);
    }
    $('#_jq_overlay_x').click( function(event) { 
        event.preventDefault(); 
        $(elms).fadeOut(500); 
    });

    $('#_jq_overlay_y').click( function(event) { 
        event.preventDefault(); 
        $(elms).fadeIn(500); 
    });
});

Here's a FIDDLE to show it working!

edited Mar 12, 2012 at 11:21

answered Mar 12, 2012 at 10:18

adeneo

319k29 gold badges410 silver badges392 bronze badges

6 Comments

Marko Over a year ago

Nope, this brokes whole preview

Johan Over a year ago

$('#_jq_overlay'+i)[0] -> $('#_jq_overlay'+i).get(0) ? Just a thought

adeneo Over a year ago

@Marko - It seems to work fine for me, added a Fiddle to my answer so you can see it affecting all the elements.

adeneo Over a year ago

@Johan - Those two do the exact same thing!

Johan Over a year ago

@adeneo I know, I just prefer the other one :-)

|

Collectives™ on Stack Overflow

Simplification of javascript code [closed]

3 Answers 3

1 Comment

4 Comments

6 Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

4 Comments

6 Comments

Related