CODEWITHSUNDEEP
c#
user1166981
user1166981

Reputation: 1746

Cycling through contents of array issue

I am using an int array to hold a long list of integers. For each element of this array I want to check if it is 1, if so do stuff relevant to 1 only, else if it is a 2, do other stuff relevant to 2, and so forth for each value stored in the array. I came up with the code below but it isn't working as expected, is there something I am missing? What is happening is that only the first value of the array is being considered. 

int[] variable1 = MyClass1.ArrayWorkings();
foreach (int i in variable1)
{ 
    if (variable1[i] == 1)
    {
        // arbitrary stuff
    }
    else if (variable1[i] ==2)
    {
        //arbitrary stuff
    }
}

Upvotes: 5

Views: 128

Answers (4)

McGarnagle
McGarnagle

Reputation: 102793

You're trying to do something that doesn't make sense. To see how it works, take a simple example, an array with values: 9, 4, 1.

If you try to run your code on this sample array, you'll get an error:

foreach (int i in variable1)
{ 
    if (variable1[i] == 1)   // the item i is 9.  
                             // But variable[i] means, get the value at position #9 in the array
                             // Since there are only 3 items in the array, you get an Out Of Range Exception
    {
        // arbitrary stuff
    }
{

Instead, this is what you need:

foreach (int i in variable1)  // i will be 9, then 4, then 1)
{ 
    if (i == 1)   
    {
        // arbitrary stuff
    }
    // ... etc
}

The alternative is to use a for loop, which would give you the indices 0, 1, and 2, like this:

for (int i=0 ; i<=variable1.Length ; i++)   // i will be 0, 1, 2
                                            // variable[i] will be 9, 4, 1
{
    if (variable1[i] == 1) 
    { 
        // stuff
    }

    // ... etc
}

Upvotes: 4

Nikhil Agrawal
Nikhil Agrawal

Reputation: 48580

The i you fetched is not the index but the value. So check i with 1 or 2.

If you were using for loop then your inner code would have worked perfectly.

int[] variable1 = MyClass1.ArrayWorkings();
foreach (int i in variable1)
{ 
    switch(i)
    {
        case 1: 
            // arbitrary stuff
        break;
        case 2: 
            //arbitrary stuff
        break;
    }
}

Try using switch case. Much faster than normal if-else.

Upvotes: 0

O. R. Mapper
O. R. Mapper

Reputation: 20770

The i in your foreach loop holds the actual element value from the array in each iteration, not the index. In your particular code sample, your array probably holds only zeroes, which is why you're only getting the first element (you're always using index 0). Hence, you should check i rather than variable1[i].

If you are going to check against various integer constants, a switch expression is more suitable, BTW:

foreach (int i in variable1) {
    switch (i) {
        case 1:
            // arbitrary stuff
            break;
        case 2:
            // arbitrary stuff
            break;
    }
}

switch/case saves you some writing; if you ever pull your values from anywhere else than from i, you can simply change the (i) part in the switch statement, and moreover, switch might be evaluated by the compiler more efficiently than the chained if-else statements.

Note: You will not be able to directly change array values in the foreach loop, as you cannot assign anything to i. If you need to assign new array values, you'll have to

  • count yourself with an additional variable while still using foreach or
  • use another loop such as for and retrieve the item at the current index yourself.

Upvotes: 8

Asken
Asken

Reputation: 8081

Write like this instead:

foreach (int i in variable1) {
    if (i == 1) {
    ....

Upvotes: 2

Related Questions