3
\$\begingroup\$

Well, i'm starting a function to 'mess' with a image gallery, i made an example

i wanna know if the way i'm doing is realy the BEST way, the function work perfect, but i fill that can be better, something more dynamic, like never flow over the edge, and mess with the number of images that user places inside the gallery, now i set as index 0 to 5.

Is there any easy way to improve this little function, make the code faster/smaller then that?

Any tips, be my guest.


jQuery

$("#ran").click(function () {
    mess(1, $(".holder img"));
});

function mess(type, gallery) {
    var getNumImg = gallery.length,
        winWidht = $("#galeria").width(),
        winheight = $("#galeria").height(),
        imgHeight = $("#content-wrapper img").height(),
        ftBottom = winheight - ($(".stats").height() + imgHeight + 150);

    $(".holder").width(winWidht / 2);

    gallery.each(function (index) {
        var randonLeft, randonRight, randonTop, randonTransform, maxIndex;
        var leftFt, topFt, transFt;
        var sign = "- ";
        var signRes = sign.charAt(Math.floor(Math.random() * sign.length));

        if (type == 1) {
            if (index == 0) {
                maxIndex = getNumImg;
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            } else if (index == 1) {
                leftFt = {
                    "from": 30,
                    "to": 40
                }
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            } else if (index == 2) {
                leftFt = {
                    "from": -30,
                    "to": -40
                }
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            } else if (index == 3) {
                leftFt = {
                    "from": -30,
                    "to": -40
                }
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            } else if (index == 4) {
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            } else if (index == 5) {
                leftFt = {
                    "from": 40,
                    "to": 50
                }
                transFt = {
                    "from": 1,
                    "to": 30
                }
                topFt = {
                    "from": 1,
                    "to": ftBottom
                }
            }
        }

        if (typeof leftFt != "undefined") {
            randonLeft = Math.floor(Math.random() * leftFt.to) + leftFt.from;
        }
        if (typeof transFt != "undefined") {
            randonTransform = Math.floor(Math.random() * transFt.to) + transFt.from;
        }
        if (typeof topFt != "undefined") {
            randonTop = Math.floor(Math.random() * topFt.to) + topFt.from;
        }

        // make it happen
        $(this).css({
            zIndex: maxIndex,
            left: randonLeft,
            top: randonTop,
            transform: "rotate(" + signRes + randonTransform + "deg)"
        });
    });
}
\$\endgroup\$
  • \$\begingroup\$ Example was not working, updated right now \$\endgroup\$ – Ricardo Arruda Mar 8 '13 at 13:13
  • \$\begingroup\$ Quer uma resposta em Port? \$\endgroup\$ – Jonny Sooter Mar 8 '13 at 15:35
2
\$\begingroup\$

I think your code is good and more importantly it works. The main thing I would change is that huge if else statement. You repeat yourself a lot there. Here's another way you could do it:

Also here's your edited fiddle.

Acho que o seu trabalho está bom e mais importante ele funciona. A parte que chama minha atenção é o if/else enorme. Você se repete muito ali. Outra forma que você pode fazer é assim:

if(type === 1){
    //You had these every time, so you can declare them once here.
    //Você declara isso em todos os casos, mas só precisa fazer uma vez aqui.
    topFt = { "from" : 1, "to" : ftBottom };
    transFt  = { "from" : 1,  "to" : 30 };

    //I Took out all the repetitions and left the cases where we actually change something.
    //Tirei todos os casos em que você estava se repetindo e deixei somente os que realmente estão mudando algo.
    switch(index){ 
        case 0:
            maxIndex = getNumImg;
            break;
        case 1:
            leftFt   = { "from" : 30, "to" : 40 }
            break;
        case 2: //If index === 2
        case 3: //OR if index === 3
            leftFt   = { "from" : -30,  "to" : -40 }
            break;
        case 5:
            leftFt   = { "from" : 40,  "to" : 50 }
            break;
        //Add more cases if you want/need to.
        //Adicione mais casos aqui se precisar.
        default:
            break;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.