Советы по коду - Как сделать более кратким (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");
        } 
});

Не беспокойтесь о том, как я делаю вычисления;Я оставил этот код вне метода, поскольку он, по сути, не имеет значения, но он есть , если вам интересно.

Я создал массив для каждого набора координат и просто вызвал их.Однако это не «прохождение» гигантского массива, заполненного всеми координатами каждой области.Можете ли вы придумать способ сделать это, или это лучшее, что я мог сделать на данный момент?

Answer 1

Я думаю, что он имеет в виду что-то вроде этого:

$("#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.

Answer 2

С первого и очень быстрого взгляда вы можете выделить область щелчка в отдельную функцию.

Примерно так.

$("#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];. Я хотел бы рассмотреть вопрос о том, чтобы сделать их глобальными переменными, чтобы они выходили за рамки функции click.

Это будет читать как

// 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 будет означать больше? Помните, что вы рассказываете историю с кодом. Чем больше вы сможете сделать, тем лучше будет читать следующий человек, используя наименьшее количество циклов мозга для чтения кода.

Вы должны постоянно совершенствовать, чтобы получить действительно чистый код.

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);
    }
}