3

コードをより簡潔にしようとしています (つまり、コードの繰り返しを減らします)。最近のコードをより簡潔にする方法についてスーパーバイザーからアドバイスを受けましたが、その方法が正確にはわかりません。

ユーザーが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");
        } 
});

計算方法は気にしないでください。そのコードは基本的に無関係なので、メソッドから除外しましたが、疑問に思っている場合に備えてあります。

座標セットごとに配列を作成し、それらを呼び出すだけです。ただし、これは、各領域のすべての座標で満たされた巨大な配列を「ループする」わけではありません。これを行う方法を思いつくことができますか、それとも現時点で私ができる最善の方法ですか?

4

3 に答える 3

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];

次のように読みます

// 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]
}

うまくいけば、私のプロセスがさらに洗練されたものの 1 つであることがわかります。メソッドを最初に作成するときに、きれいなコードを書くことを期待しないでください。後でそれを見て、それが物語を語っているかどうかを確認する必要があります. もう 1 つの変更は の名前ですdiv1_Coord_Area1。その名前は本当に変数の意図を反映していますか? いいえHotSpotArea1、もっと意味がありますか？コードでストーリーを語っていることを忘れないでください。できることが多ければ多いほど、次の人はコードを読むために使用する脳のサイクル数が少なくなります。

本当にきれいなコードを得るには、常に改良を重ねる必要があります。

于 2011-03-28T21:15:05.090 に答える
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 であっても、常に配列内のすべての項目を反復処理するため、ループを変更する必要はありません。

于 2011-03-28T21:20:29.810 に答える
1

私があなただったら、あなたのエリア オブジェクトを作成します。

// 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);
    }
}
于 2011-03-28T21:44:24.500 に答える