Ternary operator in actualization of a loop

Question

I am trying to make two loops into one. This Loop should go through the array from the beginning to the end and otherwise. However my increment is not correct. Can anyone help? Thank you in advance.

for (int i = ((asc == true) ? 0 : calendar.Length - 1);
    ((asc == true) ? i < calendar.Length : i > 0); 
    (asc==true) ? i++ : i--)

*Why* are you trying to make two loops into one? And why are you comparing with `true` instead of just using `asc` as the condition? Fundamentally, you're trying to abuse the conditional operator... — Jon Skeet, Nov 10 '14 at 19:21
I would be very upset if I saw this code in production. Just use `Reverse` for goodness sake. — BradleyDotNET, Nov 10 '14 at 19:22
Always post the full error message(s). Your code does not compile, that's not very clear from "my increment is not correct". — H H, Nov 10 '14 at 19:46
You are trying to make compact code but you have "== true" not once but 3 times? It's an entirely unnecessary clause if 'asc' is a Boolean. — Acey, Nov 10 '14 at 19:57

Jon Skeet · Answer 1 · 2014-11-10T21:39:23.000

6

Personally, I find that very hard to read, as well as invalid (it won't compile) - because you're trying to use the conditional operator as a statement expression, when it's not. Personally, I'd write something like:

for (int i = 0; i < calendar.Length; i++)
{
    int index = asc ? i : calendar.Length - 1 - i;
    // Now use index...
}

Making three different aspects all conditional feels like a nasty way to go.

edited Nov 10 '14 at 21:39

answered Nov 10 '14 at 19:23

Jon Skeet

1,421,763
867
9,128
9,194

2

I think you meant to subtract `i` at the end instead of `index`. – juharr Nov 10 '14 at 19:25
I don't see where the conditional op is used as a statement, `for(a;b;c)` requires 3 expressions. – H H Nov 10 '14 at 19:28
@Henk: the second is an expression, but I think the first and third have to be statements. Will check tonight. – Jon Skeet Nov 10 '14 at 19:32
You're kind of right, it should be "zero or more statement expressions". The OP's code does not compile, that wasn't clear. – H H Nov 10 '14 at 19:45

score 4 · Answer 2 · answered Nov 10 '14 at 19:30

4

Jon offered a good option, but if principle:

for (int i = (asc ? 0 : calendar.Length - 1);
     asc ? i < calendar.Length : i >= 0; 
     i += asc?1:-1)
{
            //body
}

answered Nov 10 '14 at 19:30

Evgeniy Mironov

777
6
22

This does fix the fundamental issue with the original code. And the irritating one of comparing a `bool` to `true`. – juharr Nov 10 '14 at 19:32
1

To be fair, you're making an assumption that it's not a comparison against a System.Nullable. – lsuarez Nov 10 '14 at 19:33

score 4 · Accepted Answer · answered Nov 10 '14 at 19:33

Look at the C# reference of "for": http://msdn.microsoft.com/en-us/library/ch45axte.aspx

Specifically:

The iterator section defines what happens after each iteration of the body of the loop. The iterator section contains zero or more of the following statement expressions, separated by commas:

assignment statement
invocation of a method
prefix or postfix increment expression, such as ++i or i++
prefix or postfix decrement expression, such as --i or i--
creation of an object by using new
await expression

The expression: "(asc==true) ? i++ : i--" is none of these things.

Therefore, you'd want the assignment: i += (asc ? 1 : -1)

for (int i = ((asc) ? 0 : calendar.Length - 1);
      ((asc) ? i < calendar.Length : i >= 0); 
      i += (asc) ? 1 : -1)

Incidentally, as pointed out in comment, you'll probably want to look at index 0 in the condition, so your condition statement in the "descending" case should be i >= 0 (reflected in the code).

1

It should also be `i >= 0`. – juharr Nov 10 '14 at 19:35

score 1 · Answer 4 · answered Nov 10 '14 at 19:26

That for loop is... odd. It is also very hard to read. In the spirit of "better ways to do this", I would suggest just using Reverse:

IEnumerable<Day> collection = asc ? calendar : calendar.Reverse();

for (int i = 0; i < calendar.Length; i++)
{
    collection.ElemantAt(i);// This is the current element
}

//Or better, you are getting everything anyways:
foreach (Day day in collection)
{
}

Ternary operator in actualization of a loop

4 Answers4