JavaScript - Using for loop and if-else statement to go through array

Question

I'm trying to loop through an array and check if the User's ip address matches one of the i.p addresses in my clientip array. After the first loop (i=0), it jumps to the else statement right away and doesn't check the other elements in the array. Any idea what is wrong? I think it's my logic that is wrong.

<script>
        $(document).ready(function(){
            for(var i = 0; clientip.length; i++){
                if(clientip[i] === userip.toString() ){
                    console.log("Your IP is :", userip);
                    $("#showButtons").show();
                    break;
                }
                else{
                    console.log("Wrong ip address");
                    console.log(userip);
                    $("#showButtons").hide();
                    alert("You are not connected to the correct IP Address");
                    break;
                };
            };
        });
    </script>

Thank you

Thanks everyone for your help!

krup
– krup

2016-04-12 19:49:38 +00:00
Commented Apr 12, 2016 at 19:49 — krup
– krup, Commented Apr 12, 2016 at 19:49

user6183958 · Accepted Answer · 2016-04-12 19:36:16Z

2

You should better try indexOf method.

So, if you change your if statement as, if(clientip.indexOf(userip.toString() ) != -1) or if(clientip.indexOf(userip.toString() ) >=0) eveything will be fine. Thus you'll not need break statement.

answered Apr 12, 2016 at 19:36

user6183958

Sign up to request clarification or add additional context in comments.

Comments

Przemysław Melnarowicz · Accepted Answer · 2016-04-12 19:31:44Z

1

You have

break;

In both if and else, so it will always break loop after first try, and for is broken...

Try:

<script>
    $(document).ready(function(){
        for(var i = 0; i < clientip.length; i++){
            if(clientip[i] === userip.toString() ){
                console.log("Your IP is :", userip);
                $("#showButtons").show();
                break;
            }
            else if (i === clientip.length -1) {
                console.log("Wrong ip address");
                console.log(userip);
                $("#showButtons").hide();
                alert("You are not connected to the correct IP Address");
                break;
            };
        };
    });
</script>

answered Apr 12, 2016 at 19:31

Przemysław Melnarowicz

1,0477 silver badges20 bronze badges

5 Comments

Dominique Fortin Over a year ago

i >= clientip.length -1 would be beter.

Przemysław Melnarowicz Over a year ago

"i >= clientip.length -1 would be beter." - i will never be greater than clientip.length

Dominique Fortin Over a year ago

@meler I erased because i was wrong and i gave you +1. So why would you give -1.

Przemysław Melnarowicz Over a year ago

@DominiqueFortin no hard feelings, everything is right now. Regards

Dominique Fortin Over a year ago

@meler It's a good practice to use >= and <= in for loop condition instead of === or !==.

Nikolai Mavrenkov · Accepted Answer · 2016-04-12 19:30:37Z

0

For loop stops after first iteration because of break stetement

answered Apr 12, 2016 at 19:30

Nikolai Mavrenkov

1,90319 silver badges22 bronze badges

2 Comments

krup Over a year ago

I removed it and it still jumps to the else statement after first iteration. i set a breakpoint at the first if statement and it jumps again back down

Nikolai Mavrenkov Over a year ago

@krup Well, it jumps to else branch, because your if condition is false. Removing break only solve this thing: doesn't check the other elements in the array. Apparently, it jumps to else branch, because first client ip isn't equals to userip :)

Arturo Montaño · Accepted Answer · 2016-04-12 19:38:06Z

0

why are you using break within your if and else statements? htry removing them

also, you may want to set a flag, lets say the 'found' variable give it the initial value of false and make it true in a if statement, in case the ip is found in your array, then , once the while is done searching, you could use your if / else statement

if(found===true ){
   console.log("Your IP is :", userip);
   $("#showButtons").show();

 }
 else{
   console.log("Wrong ip address");
   console.log(userip);
   $("#showButtons").hide();
   alert("You are not connected to the correct IP Address");
};

otherwise every iteration of your cycle would be shoing the found/ not found message

edited Apr 12, 2016 at 19:38

answered Apr 12, 2016 at 19:32

Arturo Montaño

1218 bronze badges

5 Comments

devlin carnate Over a year ago

The breaks are designed to stop the iteration when the desired value is found. Why waste time continuing the iteration if you've already found what you're looking for?

Ian McLaird Over a year ago

@devlincarnate, yes, but in the question, the asker is using break in the if and the else blocks, which means only the first iteration is ever run. There might as well not be a loop there at all.

Arturo Montaño Over a year ago

@devlincarnate even following that logic, the break should not be present on the else statement, otherwise if the ip is not found in the first position it wont be found, ever.

devlin carnate Over a year ago

@ArturoMontaño - so THAT should be your suggestion, not to just remove the breaks.

Arturo Montaño Over a year ago

Perhaps, but using break in that way si against conventions, and his code would be better written if he manages to make the if condition work instead of using breaks

Dave · Accepted Answer · 2016-04-12 19:48:09Z

clientip.length will always be truthy (unless the array is empty), so you should declare your loop like this:

for(var i = 0; clientip.length < i; i++)

The logic in the rest of your loop is kind of bizzarre. The else clause handles the case of what happens if the IP doesn't match anything in the array, but you can't know that until the loop is complete, so you shouldn't put that in there:

        var found = false;

        for(var i = 0; clientip.length; i++){
                        if(clientip[i] === userip.toString() ){
                            console.log("Your IP is :", userip);
                            $("#showButtons").show();
                            // You found a match.  You're done.
                            found = true
                            break;
                        }

                    }
// No match was found.  
   if(!found) {
       console.log("Wrong ip address");
       console.log(userip);
       $("#showButtons").hide();
       alert("You are not connected to the correct IP Address");
       break;
   }

Finally, don't put semicolons after loops or if/else blocks. At best, they don't do anything, and at worst, they'll cause you code to do something funky.

However, there are easier ways to check an array for the presense of a certain value:

if(clientip.indexOf(userip.toString()) != -1)
{
// userip was found
} else {
// userip was not found
}

Collectives™ on Stack Overflow

JavaScript - Using for loop and if-else statement to go through array

5 Answers 5

Comments

5 Comments

2 Comments

5 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

5 Comments

2 Comments

5 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related