javascript - Jquery - Carasol 构建完成并希望获得有关最佳实践/整理我的代码的建议

Question

在过去的几天里，我一直在制作自己的雨伞。

我的 Jquery 基于网络上的教程以及来自 SO 的帮助和建议。

我不是 Jquery 大师，只是一个爱好者，我认为我的代码有点草率，因此发表了这篇文章。

这里是工作代码的链接:http://jsfiddle.net/JHqBA/2/ (更新链接)

基本上发生的事情是: 如果有人在 url 中点击带有 # 值的页面，它将显示适当的幻灯片，示例为 www.hello.com#two，这将滑动到第二张幻灯片

如果有人点击数字，它会显示相应的幻灯片

next 和 prev 也在幻灯片中滑动。

问题是，有没有什么我可以写得更好的，因为我知道有很多重复的代码。

我明白这是一个很大的问题，但它会帮助我学到更多(我认为我的代码有点老派)

如果有人有任何问题，请随时提问，我会回答它做什么或应该做什么。

啪啪啪

--- 编辑 ----

我现在只做了一个动画函数，去掉了很多重复的代码。

我还没有研究功能，但很快就会研究。

我想了解更多关于在 jQuery 就绪 block 之外创建一个新函数的信息，因为我无法使它正常工作或完全理解如何让它正常工作，抱歉

在我对它感到满意之前，任何更多的提示都会非常有用，我会继续从事这个项目。

还有没有更好的写法:

if ($slideNumber == 1) {
    $('#prev').attr("class", "not_active")
    $('#next').attr("class", "active")
}
else if ($slideNumber == divSum) {
    $('#next').attr("class", "not_active");
    $('#prev').attr("class", "active");
}
else {
    $('#prev').attr("class", "active")
    $('#next').attr("class", "active")
};

完整的 Jquery:

$(document).ready(function () {

    ////////////////////////////  INITAL SET UP  /////////////////////////////////////////////

    //Get size of images, how many there are, then determin the size of the image reel.
    var divWidth = $(".window").width();
    var divSum = $(".slide").size();
    var divReelWidth = divWidth * divSum;

    //Adjust the image reel to its new size
    $(".image_reel").css({ 'width': divReelWidth });

    //set the initial not active state
    $('#prev').attr("class", "not_active");

    ////////////////////////////  SLIDER  /////////////////////////////////////////////

    //Paging + Slider Function
    rotate = function () {
        var triggerID = $slideNumber - 1; //Get number of times to slide
        var image_reelPosition = triggerID * divWidth; //Determines the distance the image reel needs to slide
        //sets the active on the next and prev
        if ($slideNumber == 1) {
            $('#prev').attr("class", "not_active")
            $('#next').attr("class", "active")
        }
        else if ($slideNumber == divSum) {
            $('#next').attr("class", "not_active");
            $('#prev').attr("class", "active");
        }
        else {
            $('#prev').attr("class", "active")
            $('#next').attr("class", "active")
        };
        //Slider Animation
        $(".image_reel").animate({
            left: -image_reelPosition
        }, 500);
    };

    ////////////////////////////  SLIDER CALLS  /////////////////////////////////////////////

    //click on numbers
    $(".paging a").click(function () {
        $active = $(this); //Activate the clicked paging
        $slideNumber = $active.attr("rel");
        rotate(); //Trigger rotation immediately
        return false; //Prevent browser jump to link anchor
    });


    //click on next button
    $('#next').click(function () {
        if (!$(".image_reel").is(':animated')) { //prevent clicking if animating
            var left_indent = parseInt($('.image_reel').css('left')) - divWidth;
            var slideNumberOn = (left_indent / divWidth);
            var slideNumber = ((slideNumberOn * -1) + 1);
            $slideNumber = slideNumber;
            if ($slideNumber <= divSum) { //do not animate if on last slide
                rotate(); //Trigger rotation immediately
            };
            return false; //Prevent browser jump to link anchor
        }
    });

    //click on prev button
    $('#prev').click(function () {
        if (!$(".image_reel").is(':animated')) { //prevent clicking if animating
            var left_indent = parseInt($('.image_reel').css('left')) - divWidth;
            var slideNumberOn = (left_indent / divWidth);
            var slideNumber = ((slideNumberOn * -1) - 1);
            $slideNumber = slideNumber;
            if ($slideNumber >= 1) { //do not animate if on first slide
                rotate(); //Trigger rotation immediately
            };
        }
        return false; //Prevent browser jump to link anchor
    });

    //URL eg:www.hello.com#one                  
    var hash = window.location.hash;
    var map = {
        one: 1,
        two: 2,
        three: 3,
        four: 4
    };
    var hashValue = map[hash.substring(1)];
    //animate if hashValue is not null
    if (hashValue != null) {
        $slideNumber = hashValue;
        rotate(); //Trigger rotation immediately
        return false; //Prevent browser jump to link anchor
    };
});

Answer 1

问答已移至https://codereview.stackexchange.com/questions/8634/jquery-carasol-build-finished-and-would-like-advice-on-best-practice-neateni/8635#8635

1) 关注点分离

首先将您的代码重构为更精细的函数。 您可以在 http://en.wikipedia.org/wiki/Separation_of_concerns 阅读更多关于 SoF 的信息

更新: 例如。不要让你的卷轴大小调整代码内联，而是把它放在它自己的函数中，就像这样:

function setImageReelWidth () {
    //Get size of images, how many there are, then determin the size of the image reel.
    var divWidth = $(".window").width();
    var divSum = $(".slide").size();
    var divReelWidth = divWidth * divSum;

    //Adjust the image reel to its new size
    $(".image_reel").css({ 'width': divReelWidth });                            
}

这实现了两件事:

一个。首先，它将逻辑上内聚的代码块分组，将其从主代码中删除，从而产生更清晰的代码栖息地。 b.它通过描述其功能的函数名称有效地为代码块提供了一个标签，从而使代码的理解变得更加简单。

之后，你也可以将整个东西封装在它自己的“类”(函数)中，你可以将它移动到它自己的js文件中。

2) jQuery“on”函数

使用“on”函数来附加您的点击事件，而不是“click”函数。 http://api.jquery.com/on/ 这还有一个额外的好处，即可以将它绑定(bind)到与您的选择器匹配的 future 元素，即使它们尚不存在。

3)就绪函数

// I like the more succinct:
$(handler)
// Instead of:
$(document).ready(handler)

但您可能喜欢更明显的语法。

这些只是开始的几件事。

-- 更新 1 --

好吧，StackOverflow 并不真正适合进行中的重构工作，但我们会凑合着做。我认为你应该在你的问题中保留你的原始代码块，以便 future 的读者可以看到它从哪里开始以及它是如何系统地改进的。

I would like to know more about the create a new function, outside of the jQuery ready block as i cant get this working or quite understand how i can get it to work sorry

我不熟悉 jsfiddle.net，但它看起来很酷而且很有帮助，但如果您不知道发生了什么，也可能会有点困惑。我不确定我这样做了 :)，但我认为脚本编辑器窗口会生成一个 .js 文件，该文件会自动被 html 文件引用。

所以这里有一个在 ready block 之外定义但从内部引用的函数示例。

function testFunction () {
    alert ('it works');
}
$(document).ready(function () {
    testFunction();

    // ... other code
});

这应该会在页面加载时弹出一个警告框，提示“它有效”。 你可以自己试试。 然后，一旦你开始工作，你就可以将其他逻辑上内聚的代码块重构为它们自己的函数。稍后您可以将它们全部包装到它们自己的 javascript“类”中。但我们会做到这一点。