mike
mike

Reputation: 749

properly subtract variable in jquery

This is literally the first few functions I've ever written...it works, but I was wondering if there was another more efficient way of limiting variable i to nothing below 0 and nothing above 5 rather than making those extra if statements in each .click().....then again the code is very minimal so maybe this is the most efficient way of limiting a variable?!

var i=0;
$(".add").click(function(event){
    if(i < 5) {
        var container = $('div');
        container.append($("<p class='blabla'>blablabla</p>"));
        if(i < 5){i++}
    }
});

$(".delete").click(function(event){
    if(i < 5, i > 0){i--}
    $("div p:last-of-type").remove();
});

Upvotes: 0

Views: 1057

Answers (4)

williambq
williambq

Reputation: 1125

Instead of counting, use a max value you want to control and let jQuery do the rest:

http://jsfiddle.net/3D69b/

<a class='add'>Add</a><br/>
<a class='delete'>Del</a>
<div class='add'>AppendToMe</div>

Then:

var gv_nMax = 5;

$(".add").click(function(event){
    if($("div p").length < gv_nMax) 
    $('div').append($("<p class='blabla'>blablabla</p>"))
});
$(".delete").click(function(event){ $("div p:last-of-type").remove(); });

Upvotes: 1

gvee
gvee

Reputation: 17161

In your example you actually don't need the variable i as you can simply count the number of elements.

$(".add").click(function () {
    if ($('div').children('p').length < 5) {
        $('div').append('<p>item</p>');
    }
});

or

$(".add").click(function () {
    if ($('div p').length < 5) {
        $('div').append('<p>item</p>');
    }
});

And your remove function doesn't need to check for the number of items at all as the selector just won't find anything to remove.

$(".delete").click(function () {
    $('div p:last-of-type').remove();
});

Upvotes: 1

Bergi
Bergi

Reputation: 664630

making those extra if statements in each .click()

They're necessary and fine. However, you can shorten them a little.

….append($("<p class='" + name + " select'>" + name + "</p>"));

The append method does accept plain HTML strings as well, you don't need to wrap them in a $() invocation.

if(i < 5) {
    if(i < 5){i++}
}

The inner if condition is superfluous. Nothing has changed i in between, so you don't need to check again.

if(i < 5, i > 0){i--}

Here you're actually using the comma operator, which only yields the right operand to take effect in the effect. You probably wanted an && AND operator, but you should not check for i<5 at all - it won't get outside the range anyway and you don't want to duplicate the upper boundary.

So use

var i=0;
$(".add").click(function(event){
    if (i < 5) {
        var container = $('div');
        container.append("<p class='blabla'>blablabla</p>");
        i++;
    }
});
$(".delete").click(function(event){
    if (i > 0) {
        $("div p:last-of-type").remove();
        i--;
    }
});

Another approach, without the counter variable i, would be to count the existing elements each time:

$(".add").click(function(event){
    var container = $('div');
    if (container.find("p").length < 5) {
        container.append("<p class='blabla'>blablabla</p>");
    }
});
$(".delete").click(function(event){
    $("div p:last-of-type").remove(); // if nothing is found nothing is removed anyway
});

Upvotes: 1

McGarnagle
McGarnagle

Reputation: 102763

You don't need the second check in if (i<5) i++; (that block wouldn't have been entered if i was 5 or more).

Also, You can just write if(i > 0) i--;, because it doesn't matter there whether i is more than 5.

Fiddle.

Upvotes: 2

Related Questions