JavaScript if Statement

Question

I'm creating an if statement that fires different functions. Is there a better way I can accomplish this functionality?

$("#slide01-bttn").click(function() {

    if ($("#slide-1").is('#featured_ul li:nth-child(1)'))
    {
    alert("First Child");
    moveSlideFirstChild();
    }

    if ($("#slide-1").is('#featured_ul li:nth-child(2)'))
    {
    alert("Second Child");
    moveSlideSecondChild();
    }

    if ($("#slide-1").is('#featured_ul li:nth-child(3)'))
    {
    alert("Third Child");
    moveSlideThirdChild();
    }

    if ($("#slide-1").is('#featured_ul li:nth-child(4)'))
    {
    alert("Fourth Child");
    moveSlideFourthChild();
    }
});

If `#slide-1` will always be one of those li elements then you could use the [`.index()` method](http://api.jquery.com/index/) to see which li it is, and combine the `moveX()` functions to a single, more generic function that works from an index passed as a parameter... — nnnnnn, Sep 26 '12 at 13:36
Could you please show us the `moveSlide…Child` function? I guess it would be much easier to give that one an argument. — Bergi, Sep 26 '12 at 13:37
probably, any chance you could post the html & moveSlide[number]Child() code so we can have a better idea of what's going on? — SubjectCurio, Sep 26 '12 at 13:44

Bergi · Answer 1 · 2012-09-26T13:56:53.113

2

$("#slide01-bttn").click(function() {
    var slide = $("#slide-1");
    var index = $('#featured_ul li').index(slide);
    var position = ["First", "Second", "Third", "Fourth"][index];
    if (position) {
        alert(position+" Child");
        // assuming they are global functions:
        window["moveSlide"+position+"Child"]();
    }
});

If your functions are local variables, you still could do

    var fn = [moveSlideFirstChild, moveSlideSecondChild, moveSlideThirdChild, moveSlideFourthChild][index];
    if (fn) fn();

But I would really recommend to paramatize your moveSlide function, so that you only need one that takes the element to move as an argument:

    moveSlide(slide);

edited Sep 26 '12 at 13:56

answered Sep 26 '12 at 13:44

Bergi

630,263
148
957
1,375

Nice idea but **global scope**? – Robert Koritnik Sep 26 '12 at 13:49
Global methods are attached to the window object and can be executed this: window.moveSlide(slidenum); – Rob Angelier Sep 26 '12 at 13:51
I did not said I would like it :-) It is [not possible for local variables](http://stackoverflow.com/q/39960/1048572) - you should use an object to store the functions if they really needed to be different – Bergi Sep 26 '12 at 13:51
you could store those functions inside a local variable you know instead of polluting global scope. – Robert Koritnik Sep 26 '12 at 13:56
@RobertKoritnik: I know, I just guessed that the OP did pollute it :-) I already mentioned objects to store them by-name, and my answer uses an array of them… – Bergi Sep 26 '12 at 14:00
@RobAngelier: `moveSlide(slidenum)` only works for abstraction, which implies the poster should modify his other four functions - my answer is for that, but I can't say if it's right. Bergi's answer shows a cleaner way to call the four functions (`moveSlideFirstChild()...moveSlideFourthChild()`), without the author having to modify his other code, which using the `window` object is useful. He could have also used an `eval` or other methods to accomplish the same type of dynamic call. – vol7ron Sep 26 '12 at 17:39
@vol7ron that's true, but i try to provide a solution that fits better. Why should you have 4 functions that do exactly the same? (if thats thats the case). In some cases i have to agree with you! We are all here to learn something right? :-) – Rob Angelier Sep 27 '12 at 05:40
Yes, and I was surprised to see so many different answers to a seemingly simple question - all right in their own merit. – vol7ron Sep 27 '12 at 06:52

Robert Koritnik · Accepted Answer · 2012-09-26T13:54:19.043

2

You could of course simplify your code to this

var pos = $("#slide-1").prevAll().length;
switch (pos)
{
    case 0:
        ...
        break;
    case 1:
        ...
        break;
    ...
    default:
        ...
        break;
}

But it seems that you should optimize your methods that move child elements to have just one function instead of N so you could do a simple call:

var elem = $("#slide-1");

// pass position AND element because you'll likely use it inside
moveSlideChild(elem.prevAll().length, elem);

You could of course just pass in the element and get position inside. And save some element selections because you're doing many many jQuery element selector calls. This would make your code significantly faster and optimised.

edited Sep 26 '12 at 13:54

answered Sep 26 '12 at 13:45

Robert Koritnik

103,639
52
277
404

@RobAngelier: As Forrest Gump would say: *Shit happens* ;) – Robert Koritnik Sep 26 '12 at 13:47
@Robert Koritnik thanks for you help. I'm sure there are multiple ways to accomplish this. – Nick Rivers Sep 27 '12 at 16:52

score 1 · Answer 3 · answered Sep 26 '12 at 13:39

1

I think the best thing that you could do would be to paramatize your moveSlide functions

function moveSlide(childNumber) {

    //Grab the child element you are looking for
    //Move it code
}

Really you should be able to do something with the ids on the li to simplify it even more. I'm not sure how the HTML is layed out. if you could provide a fiddle we could probably make more progress.

answered Sep 26 '12 at 13:39

Dameo

334
2
7

1

pedant "parameterize" /pedant – jbabey Sep 26 '12 at 14:01

Rob Angelier · Answer 4 · 2012-09-26T13:53:30.253

1

You could use a switch to get the job done:

$("#slide01-bttn").click(function() {
var pos = $("#slide-1").prevAll().length;
switch(pos)
{
case 0:
case 1:
  moveSlide(pos);
  break;
default:
  //code to be executed if pos is different from case 0 and 1
}
}

function moveSlide(childNumber) {

    //Grab the child element you are looking for
    //Move it code
}

edited Sep 26 '12 at 13:53

answered Sep 26 '12 at 13:45

Rob Angelier

2,335
16
29

score 0 · Answer 5 · answered Sep 26 '12 at 13:49

If moveSlideNChild() all do similar things, it would be best to apply some abstraction to reduce the amount of code. You can then call

var $list  = $('#featured_ul li');             // set here for cacheing
var $slide = $("#slide-1");                    // set here for cacheing

$("#slide01-bttn").click(function() {
    var position = $list.index($slide);
    if( position >= 1 && position <= 4) {      // only want 1 through 4
        alert('Child ' + position);
        moveSlideChild(position);
    }
}

JavaScript if Statement

5 Answers5