代码建议 - 如何使代码更加简洁 (Javascript/Jquery)

Question

我正在尝试使我的代码更加简洁（即减少重复的代码）。 我从我的主管那里得到了一些建议，以使我的最新代码更加简洁，但我不确定如何做到这一点。

我有一些坐标，用于检查用户是否点击了 div 的某个特定区域。 我被告知应该将所有坐标放入数组中，并在需要时“循环获取”它们。 我 -想- 理解他的建议，但由于我还没有足够的编程经验，所以我无法完全理解。以下是我迄今为止所做的，以便您更好地了解情况：

    $("#div1").click(function(e)
    {
        // Arrays containing the x and y values of the rectangular area around a farm
        // [minX, maxX, minY, maxY]
        var div1_Coord_Area1= [565, 747, 310, 423];
        var div1_Coord_Area2= [755, 947, 601, 715];

        if(areaX >= Reg2_Coord_Farm1[0] && areaX <= Reg2_Coord_Farm1[1] && areaY >= Reg2_Coord_Farm1[2] && areaY <= Reg2_Coord_Farm1[3]) 
        {
            alert("You clicked in the first area");
        }
        if(areaX >= Reg2_Coord_Farm2[0] && areaX <= Reg2_Coord_Farm2[1] && areaY >= Reg2_Coord_Farm2[2] && areaY <= Reg2_Coord_Farm2[3]) 
        {
            alert("You clicked in the second area");
        } 
});

不用担心我是如何进行计算的，因为这段代码基本上与此无关，但是如果你想知道，它是存在的。

我为每组坐标创建了一个数组，并简单地调用它们。然而，这并不是“循环遍历”填充有每个区域所有坐标的巨大数组。你能想到一种更好的方法吗？或者这是目前我所能做到的最好的方法了？

- Briz

Answer 1

从第一眼和非常快速的扫视中，您可以将点击区域提取到一个单独的函数中。

就像这样。

$("#div1").click(function(e)
{
    // Arrays containing the x and y values of the rectangular area around a farm
    // [minX, maxX, minY, maxY]
    var div1_Coord_Area1= [565, 747, 310, 423];
    var div1_Coord_Area2= [755, 947, 601, 715];

    if(inArea(div1_Coord_Area1, someStructForMouseLocation)) 
    {
        alert("You clicked in the first area");
    }
    if(inArea(div1_Coord_Area2, someStructForMouseLocation)) 
    {
        alert("You clicked in the second area");
    } 
});

function inArea(coordArea, mouseLocation)
{
    return mouseLocation.X >= coordArea[0] && mouseLocation.X <= coordArea[1] && mouseLocation.Y >= coordArea[2] && mouseLocation.Y <= coordArea[3]
}

你的代码中似乎存在一些“神奇数字”：var div1_Coord_Area1= [565, 747, 310, 423]; 和 var div1_Coord_Area2= [755, 947, 601, 715];。建议将它们定义为全局变量，这样就可以超出点击函数的范围。

修改后代码如下：

// Arrays containing the x and y values of the rectangular area around a farm
// [minX, maxX, minY, maxY]
var div1_Coord_Area1= [565, 747, 310, 423];
var div1_Coord_Area2= [755, 947, 601, 715];

$("#div1").click(function(e)
{
    if(inArea(div1_Coord_Area1, someStructForMouseLocation)) 
    {
        alert("You clicked in the first area");
    }
    if(inArea(div1_Coord_Area2, someStructForMouseLocation)) 
    {
        alert("You clicked in the second area");
    } 
});

function inArea(coordArea, mouseLocation)
{
    return mouseLocation.X >= coordArea[0] && mouseLocation.X <= coordArea[1] && mouseLocation.Y >= coordArea[2] && mouseLocation.Y <= coordArea[3]
}

希望你能看到我的过程是一个进一步完善的过程。刚开始创建方法时，不要期望写出干净的代码。你必须之后再看一下它是否讲述了一个故事。另一个改变是 div1_Coord_Area1 的名称，这个名字真的反映了变量的意图吗？不是的。使用 HotSpotArea1 会更好吗？记住，你正在用代码讲述一个故事。你讲得越好，下一个人阅读代码所需的大脑运转次数就越少。

你必须不断完善才能得到真正干净的代码。

- David Basarab

---

这个可能行。我会研究一下如何将其实现到我的代码中，并让你知道我发现了什么。 - Briz

我很感激额外的建议和代码。我要说，我有名为div1_Coord_Area1的名称，因为我有不止一个div，并且每个div中有更多的区域。 - Briz

Answer 2

我认为他的意思更像是这样：

我想他所指的是类似于这样的东西：

$("#div1").click(function(e){
    // Arrays containing the x and y values of the rectangular area around a farm
    // For each array: [area1, area2, area3, ... areaX]
    var minX = [565, 755];
    var maxX = [747, 947];
    var minY = [310, 601];
    var maxY = [423, 715];

    for (var i = 0; i < minX.length; i++) {
      if (areaX >= minX[i] && areaX <= maxX[i] && areaY >= minY[i] && areaY <= maxY[i]) {
        alert("You clicked in area " + (i+1));
      }
    }
});

这样，你可以有更多的区域需要检查，但是永远不必更改循环，因为它将始终遍历数组中的所有项目，无论是像上面那样的2个项目，还是10个、100个项目。

- mVChr

---

这个也看起来不错。我会研究这两个建议，看哪一个对我最有效。 - Briz

这个答案对我来说效果最好，它让我的代码更加简洁，我认为它做到了我的主管想要的。非常感谢！ - Briz

Answer 3

如果我是你，我会将你的区域变成对象：

// think of this as your ClickArea class
function ClickArea(minX, maxX, minY, maxY, description) {
    this.minX = minX;
    this.maxX = maxX;
    this.minY = minY;
    this.maxY = maxY;
    this.description = description;
};
ClickArea.prototype.isInside = function(areaX, areaY) {
    return (areaX >= this.minX && areaX <= this.maxX && areaY >= this.minY && areaY <= this.maxY);
};

现在，您可以像这样创建一个ClickArea对象：

var area = new ClickArea(565, 747, 310, 423, "Area 1");

但是你会希望它们在一个数组中，所以我建议像这样创建它们：

var areas = [
    new ClickArea(565, 747, 310, 423, "Area 1"),
    new ClickArea(365, 745, 330, 423, "Area 2")
];
// you can also add new ClickAreas using the array's push method:
areas.push(new ClickArea(333, 444, 555, 566, "Area 3"));

然后您可以使用for循环对它们进行循环：

for(var i = 0; i < areas.length; i++) {
    if(areas[i].isInside(areaX, areaY)) {
        alert("You clicked in " + areas[i].description);
    }
}

- typeof

---

谢谢您的建议，但我现在没有时间完全重做代码。不过我会将其保留供将来参考。 - Briz

我明白了。将来，这将提供更大的灵活性，因为您可以向对象添加更多方法--reposition(), scale()等等... - typeof