tags:

views:

503

answers:

12

I often find myself doing the following index-counter messiness in a foreach loop to find out if I am on the first element or not. Is there a more elegant way to do this in C#, something along the lines of if(this.foreach.Pass == 1) etc.?

int index = 0;
foreach (var websitePage in websitePages)
{
    if(index == 0)
        classAttributePart = " class=\"first\"";
    sb.AppendLine(String.Format("<li" + classAttributePart + "><a href=\"{0}\">{1}</a></li>", websitePage.GetFileName(), websitePage.Title));
    index++;
}
+4  A: 
if (websitePages.IndexOf(websitePage) == 0)
    classAttributePart = " class=\"last\"";

This may be more elegant, but it will probably be less performant since it has to check the index of each element.

Joel Potter
+4  A: 

This might be slightly better

bool doInit = true;
foreach (var websitePage in websitePages)
{
    if (doInit)
    {
        classAttributePart = " class=\"first\"";
        doInit = false;
    }
    sb.AppendLine(String.Format("<li" + classAttributePart + "><a href=\"{0}\">{1}</a></li>", websitePage.GetFileName(), websitePage.Title));
}

I end up doing this sort of thing a lot too, and it bugs me also.

John Knoeller
You'd better put doInit = false; in the body of the if so it won't be assigned every time. Of course this kind of performance optimization doesn't really matter for in web environment.
Stilgar
@Stilgar: you are right, normally I would have but I was mimic-ing his original code. I'll change it.
John Knoeller
I was considering suggesting this approach too, but it is just about as verbose as the original, and it only works for the very first element.
Joel Potter
+11  A: 

Slightly less verbose:

string classAttributePart = " class=\"first\"";
foreach (var websitePage in websitePages)
{
    sb.AppendLine(String.Format("<li" + classAttributePart + "><a href=\"{0}\">{1}</a></li>", websitePage.GetFileName(), websitePage.Title));
    classAttributePart = string.Empty;
}

If you are using .NET 3.5 you could use the overload of Select that gives you the index and test that. Then you also wouldn't need the StringBuilder. Here's the code for that:

string[] s = websitePages.Select((websitePage, i) =>
        String.Format("<li{0}><a href=\"{1}\">{2}</a></li>\n",
                      i == 0 ? " class=\"first\"" : "",
                      websitePage.GetFileName(),
                      websitePage.Title)).ToArray();

string result = string.Join("", s);

It looks a bit more verbose, but that's mainly because I broke the very long line many shorter ones.

Mark Byers
Despite being a great fan of LINQ I cannot agree that this code is better than the original. I doubt there is any developer on Earth who will find it easier to read.
Stilgar
+1 for the first suggestion though it only works if the index of interest is 0.
Joel Potter
+1 on the first suggestion. Another benefit of it is that it removes the possibility of a mistake inside the loop causing the 'init' stuff to be done an inappropriate number of times.
kyoryu
A: 

How about this?

var iter = websitePages.GetEnumerator();
iter.MoveNext();
//Do stuff with the first element
do {
    var websitePage = iter.Current;
    //For each element (including the first)...
} while (iter.MoveNext());
Anon.
+4  A: 

You could use a for loop instead of a foreach loop. In that case your for loop could start it's index at 1 and you could do the first element outside of the loop if the length is more than 0.

At least in this case you wouldn't be doing an extra comparison on each iteration.

Brian R. Bondy
Yeah, I'm wondering why everyone else seems stuck on using the foreach with a manual count/flag to find the first one? Is doing it that way still quicker than using a straight for loop or something?
slugster
I'm sure less efficient if you are going to use an iterator plus keep a count.
Brian R. Bondy
You can't use a for-loop if all you have is an IEnumerable, or some other class that doesn't support access by index. Like a linked list, for example.
Nevermind
Yes in some cases this answer will not apply.
Brian R. Bondy
+2  A: 

You can do it before the foreach loop if your only doing it for the first index.

Dave18
+11  A: 

Another approach is to accept that the "ugly part" has to be implemented somewhere and provide an abstraction that hides the "ugly part" so that you don't have to repeat it in multiple places and can focus on the specific algorithm. This can be done using C# lambda expressions (or using C# 2.0 anonymous delegates if you're restricted to .NET 2.0):

void ForEachWithFirst<T>(IEnumerable<T> en, 
     Action<T> firstRun, Action<T> nextRun) {
  bool first = true;
  foreach(var e in en) {
    if (first) { first = false; firstRun(e); } else nextRun(e);
  }
}

Now you can use this reusable method to implement your algorithm like this:

ForEachWithFirst(websitePages,
  (wp => sb.AppendLine(String.Format("<li class=\"first\">" +
         "<a href=\"{0}\">{1}</a></li>", wp.GetFileName(), wp.Title)))
  (wp => sb.AppendLine(String.Format("<li>" + 
         "<a href=\"{0}\">{1}</a></li>", wp.GetFileName(), wp.Title))) );

You could design the abstraction differently depending on the exact repeating pattern. The good thing is that - thanks to lambda expression - the structure of the abstraction is completely up to you.

Tomas Petricek
For readability reasons, I would rather stick with the index method from the original post. Generally, I do not like lambda expressions too much because they are often hard to read, unless they are really well formatted (or one-liners). I hardly use them (except for the IEnumerable<T> extension methods), and in the case above, they make things more complicated, I think. Just to save one or two lines of code. However, this is my personal opinion, and I am sure, there are other opinions out there. :)
gehho
+3  A: 

Another approach would be to use jQuery's first selector to set the class instead of server side code.

$(document).ready(function(){ 
     $("#yourListId li:first").addClass("first");
}
jrummell
+1 because this is helpful in my particular instance of generating website code, didn't know how to do this in CSS, nice reminder that jquery can do it, may switch it
Edward Tanguay
Glad to help. This is the kind of thing jQuery (and other javascript frameworks) is designed for.
jrummell
A: 

As my dad likes to say, "Use the right tool for the job".

In this case, look at what it is your loop needs to do.

  • If it's something that can be done outside the loop, move it and your use of foreach is fine.
  • If you need to handle a complex combination of cases, then you may want to consider using a different pattern (eg: a state pattern, or whatever is most appropriate) for the stuff you want to do inside loop, in which case you choose the loop construct that makes the most sense at the time.
  • If your loop is dependant on being able to extract the iteration index out to pass it elsewhere, then perhaps a for loop may be a better choice.

Otherwise, to answer you question, there doesn't appear to be an easy non-verbose means to identify the index of a list item as it is accessed in a foreach loop.

S.Robins
A: 
public static class ExtenstionMethods
{
    public static IEnumerable<KeyValuePair<Int32, T>> Indexed<T>(this IEnumerable<T> collection)
    {
        Int32 index = 0;

        foreach (var value in collection)
        {
            yield return new KeyValuePair<Int32, T>(index, value);
            ++index;
        }
    }
}

foreach (var iter in websitePages.Indexed())
{
    var websitePage = iter.Value;
    if(iter.Key == 0) classAttributePart = " class=\"first\"";
    sb.AppendLine(String.Format("<li" + classAttributePart + "><a href=\"{0}\">{1}</a></li>", websitePage.GetFileName(), websitePage.Title));
}
FrantzX
hmmm... I'm not a fan.
Alex Baranosky
A: 

In this example you can get rid of the index check this way.

foreach (var websitePage in websitePages) 
{ 
    classAttributePart =  classAttributePart ?? " class=\"first\""; 
    sb.AppendLine(String.Format("<li" + classAttributePart + "><a href=\"{0}\">{1}</a></li>", websitePage.GetFileName(), websitePage.Title)); 
} 

It would be better to check on the resultant data variable to perform this kind of task. In this case It checks for null of the classAttributePart string and append the initial value.

Ramesh Vel
+2  A: 

If you're interested in the first element only, the best (as in most readable) way is to use LINQ to find out what element is the first. Like this:

var first = collection.First();
// do something with first element
....
foreach(var item in collection){
    // do whatever you need with every element
    ....
    if(item==first){
        // and you can still do special processing here provided there are no duplicates
    }
}

If you need numerical value of the index, or non-first index, you can always do

foreach (var pair in collection.Select((item,index)=>new{item,index}))
{
    // do whatever you need with every element
    ....
    if (pair.index == 5)
    {
        // special processing for 5-th element. If you need to do this, your design is bad bad bad
    }
}

PS And the very best way is to use a for-loop. Use foreach only if for is not available (i.e. the collection is IEnumerable and not a list or something)

Nevermind