views:

1051

answers:

18

A colleague once said that God is killing a kitten every time I write a for-loop.

When asked how to avoid for-loops, his answer was to use a functional language. However, if you are stuck with a non-functional language, say C#, what techniques are there to avoid for-loops or to get rid of them by refactoring? With lambda expressions and LINQ perhaps? If so, how?

Questions

So the question boils down to:

  1. Why are for-loops bad? Or, in what context are for-loops to avoid and why?
  2. Can you provide C# code examples of how it looks before, i.e. with a loop, and afterwards without a loop?
+3  A: 

Your colleague is not right. For loops are not bad per se. They are clean, readable and not particularly error prone.

Daniel Daranas
+9  A: 

For loops don't kill people (or kittens, or puppies, or tribbles). People kill people. For loops, in and of themselves, are not bad. However, like anything else, it's how you use them that can be bad.

Muad'Dib
For loops don't kill people, but goto certainly does. http://xkcd.com/292/
Petar Minchev
@Muad He was talking about killing kittens, not people. You are way off topic, j/k +1
AaronLS
But do people kill kittens? Only Harkonnens, perhaps.
Charles
lol, true, I was paraphrasing the popular saying about firearms.
Muad'Dib
@charles the fremen kill harkonen, not kittens.
Muad'Dib
+1 for tribbles
Matthew Whited
I meant perhaps the Harkonnen kill kittens. But do kittens kill anyone?
Charles
I would say it depends on the context:) http://stackoverflow.com/questions/2640236/should-we-retire-the-term-context
Petar Minchev
They can kill readability though, list.Contains("foo") vs a for loop for example.
Douglas
@ Charles of course the Harkonen kill kittens(and puppies, and tribbles). they wouldn't be Harkonen if they didn't.
Muad'Dib
@Minchev: GOTO doesn't kill people, *unconstrained use of GOTO* does. See http://david.tribble.com/text/goto.html.
Loadmaster
@Petar: `for` loops are the new `goto`. :)
missingfaktor
+22  A: 

Functional constructs often express your intent more clearly than for-loops in cases where you operate on some data set and want to transform, filter or aggregate the elements.

Loops are very appropriate when you want to repeatedly execute some action.


For example

int x = array.Sum();

much more clearly expresses your intent than

int x = 0;
for (int i = 0; i < array.Length; i++)
{
    x += array[i];
}
dtb
+1 This is similar to what I mentioned regarding set based operations being bad for for-loops.
AaronLS
Yes, but how would you expect `Sum` to be implemented? This is really an argument for using methods, not avoiding `for` loops altogether.
Dan Tao
@Dan Tao: I was referring to the LINQ Sum method. Of course, at some place there will have to be a loop. But we can build us a nice set of methods (such as provided by LINQ) that abstracts the loops away and makes our code much more readable.
dtb
@dtb: I completely agree; my point was just that if the OP's coworker was trying to argue that `for` loops should be done away with completely, he/she was probably severely mistaken about how methods such as `Sum`, etc. actually work (i.e., by using `for` loops).
Dan Tao
@Dan Tao: well you *could* implement `Sum` using a for-loop, or you could do it using recursion ;) Of course, if you focus on the implementation details, you're going to really miss the point. For-loops are an anti-pattern because they aren't composable like higher-order functions.
Juliet
+1 though you should also add an example where a for-loop makes sense :)
BlueRaja - Danny Pflughoeft
@Juliet: If you focus on the implementation details, presumably it's because you're implementing something. We're not all simply consumers of pre-existing libraries that abstract all implementation details away from us; we also create our *own* abstractions, and implement our *own* functionality, and we sometimes use `for` loops (*or* recursion--though I would seriously question the motives of anyone who implemented a `Sum` method using recursion!) to do so. To me, calling `for` loops an "anti-pattern" is like saying that we should abandon nails because we can't build cities out of them.
Dan Tao
@Dan Tao @Juliet: That's actually the method Haskell uses (recursion) and it's quite elegant and "functional" after you wrap your head around it.
RCIX
@DanTao: Implementing sum using recursion is no different from using an iterative approach when considering proper use of tail recursion. Sum is just a special case of a more general sequence scan and can be efficiently implemented as such.You don't need a functional language for that,it can be implemented in C# just fine (using functional idioms of course).
Johannes Rudolph
Recursion is fine, *as long as you have enough stack space* (or the compiler can convert it into an interative loop automagically).
Loadmaster
+1  A: 

Sometimes a for-loop is bad if there exists a more efficient alternative. Such as searching, where it might be more efficient to sort a list and then use quicksort or binary sort. Or when you are iterating over items in a database. It is usually much more efficient to use set-based operations in a database instead of iterating over the items.

Otherwise if the for-loop, especially a for-each makes the most sense and is readable, then I would go with that rather than rafactor it into something that isn't as intuitive. I personally don't believe in these religious sounding "always do it this way, because that is the only way". Rather it is better to have guidelines, and understand in what scenarios it is appropriate to apply those guidelines. It is good that you ask the Why's!

AaronLS
+2  A: 

A simple (and pointless really) example:

Loop

// mystrings is a string array
List<string> myList = new List<string>();
foreach(string s in mystrings)
{
    if(s.Length > 5)
    {
        myList.add(s);
    }
}

Linq

// mystrings is a string array
List<string> myList = mystrings.Where<string>(t => t.Length > 5).ToList<string>();

In my book, the second one looks a lot tidier and simpler, though there's nothing wrong with the first one.

AndyC
Where<string>() returns IEnumerable<string>, not List<String>. Not to matter, you can write your example as var myList = myStrings.Where(t => t.Length > 5); anyway.
Douglas
I fixed it before I saw this comment, but nicely spotted! :)
AndyC
You can leave out the `<string>` annotations on your LINQ methods for more concise code - the compiler is smart enough to figure out what you mean.
Joel Mueller
+9  A: 
  1. For loops are not bad. There are many very valid reasons to keep a for loop.
  2. You can often "avoid" a for loop by reworking it using LINQ in C#, which provides a more declarative syntax. This can be good or bad depending on the situation:

Compare the following:

var collection = GetMyCollection();
for(int i=0;i<collection.Count;++i)
{
     if(collection[i].MyValue == someValue)
          return collection[i];
}

vs foreach:

var collection = GetMyCollection();
foreach(var item in collection)
{
     if(item.MyValue == someValue)
          return item;
}

vs. LINQ:

var collection = GetMyCollection();
return collection.FirstOrDefault(item => item.MyValue == someValue);

Personally, all three options have their place, and I use them all. It's a matter of using the most appropriate option for your scenario.

Reed Copsey
Thanks for the nice code samples! How do I know which is appropriate in which scenario?
Lernkurve
@Lernkurve: Personally, I choose based on the following criteria (in order): Correctness, readability, maintainability. Whichever is the easiest to understand tends to be the best choice of the three... (For loops are only really needed if you **need** the index, otherwise, use foreach or LINQ, IMO.)
Reed Copsey
I agree with using For loops only when the index is necessary. But when would you pick LINQ versus foreach?
ccomet
If you *need* the index you don't necessarily need a for loop. Select((x,i) => ...) works just fine in many cases.
Hightechrider
@ccomet: When it's more clear. Typically, for me, this means any time the for loop is trying to either "find" an element, filter, or perform one map/accumulation operation. If multiple operations occur inside the block, foreach is often cleaner.
Reed Copsey
@Reed I definitely agree regarding multiple operations. When you have a complex task you need to subdivide it into managable procedural chunks. If you try to cram it all into one LINQ statement it becomes near impossible for anyone other than the author to decipher it, and even then the author might have trouble understanding it a year later.
AaronLS
@ccomet, he really provided a good example above. If you want to perform an action on each of the elements in a collection, a foreach loop is appropriate. However, if all you want to do is extract the object(s) that match a given criteria, then LINQ is a good choice. You would typically find a foreach occuring on the LINQ results later in the code, as a matter of fact.
Anthony Pegram
These days I mostly use functional syntax and fall back to for-loops when:-A. The body of the loop contains complex logic that cannot be disentangled into a cleaner sequential application of functions and it simply easier to just write a for-loop with the complex conditional code in it.B. The task is inherently not functional, i.e. has side effectsC. The task needs exception handling in it. Sure you can write big lambda blocks with try catch in them but at some point it becomes easier and cleaner just to use a for-loop.
Hightechrider
@Reed @Anthony Thanks for the clarification. I haven't had enough LINQ experience so it's worth a good learning.
ccomet
Many times when multiple operations occur inside the block it can be easily restructured as a sequence of function applications. It's only when those multiple operations interact in a non-functional way or have side effects that you need to use a procedural loop. Just having multiple operations isn't a reason to go procedural.
Hightechrider
@Hightechrider: True, however, many times, going "functional" for the sake of going functional just hurts maintainability, as well. As I said, for me, it's all about being correct then maintainable (I love declarative programming, so I use it when it makes sense...)
Reed Copsey
+1  A: 

For loop is, let's say, "bad" as it implies branch prediction in CPU, and possibly performance decrease when branch prediction miss.

But CPU (having a branch prediction accuracy of 97%) and compiler with tecniques like loop unrolling, make loop performance reduction negligible.

digEmAll
Since most CPUs don't have instructions to operate on entire sets of data, the functional code normally turns into a loop anyway, so there's rarely (if ever) an improvement in efficiency.
Jerry Coffin
You're right.Plus, I'd like to clarify that LINQ is not functional, it uses loops anyway.
digEmAll
+3  A: 

You can refactor your code well enough so that you won't see them often. A good function name is definitely more readable that a for loop.

Taking the example from AndyC :

Loop

// mystrings is a string array
List<string> myList = new List<string>();
foreach(string s in mystrings)
{
    if(s.Length > 5)
    {
        myList.add(s);
    }
}

Linq

// mystrings is a string array
List<string> myList = mystrings.Where<string>(t => t.Length > 5)
                               .ToList<string();

Wheter you use the first or the second version inside your function, It's easier to read

var filteredList = myList.GetStringLongerThan(5);

Now that's an overly simple example, but you get my point.

Stephane
Good point, thanks.
Lernkurve
A: 

Any construct in any language is there for a reason. It's a tool to be used to accomplish a task. Means to an end. In every case, there are manners in which to use it appropriately, that is, in a clear and concise way and within the spirit of the language AND manners to abuse it. This applies to the much-misaligned goto statement as well as to your for loop conundrum, as well as while, do-while, switch/case, if-then-else, etc. If the for loop is the right tool for what you're doing, USE IT and your colleague will need to come to terms with your design decision.

Jesse C. Slicer
+3  A: 

Your colleague is wrong about for loops being bad in all cases, but correct that they can be rewritten functionally.

Say you have an extension method that looks like this:

void ForEach<T>(this IEnumerable<T> collection, Action <T> action)
{
    foreach(T item in collection)
    {
        action(item)
    }
}

Then you can write a loop like this:

mycollection.ForEach(x => x.DoStuff());

This may not be very useful now. But if you then replace your implementation of the ForEach extension method for use a multi threaded approach then you gain the advantages of parallelism.

This obviously isn't always going to work, this implementation only works if the loop iterations are completely independent of each other, but it can be useful.

Also: always be wary of people who say some programming construct is always wrong.

Jack Ryan
+1  A: 

It depends upon what is in the loop but he/she may be referring to a recursive function

    //this is the recursive function
    public static void getDirsFiles(DirectoryInfo d)
    {
        //create an array of files using FileInfo object
        FileInfo [] files;
        //get all files for the current directory
        files = d.GetFiles("*.*");

        //iterate through the directory and print the files
        foreach (FileInfo file in files)
        {
            //get details of each file using file object
            String fileName = file.FullName;
            String fileSize = file.Length.ToString();
            String fileExtension =file.Extension;
            String fileCreated = file.LastWriteTime.ToString();

            io.WriteLine(fileName + " " + fileSize + 
               " " + fileExtension + " " + fileCreated);
        }

        //get sub-folders for the current directory
        DirectoryInfo [] dirs = d.GetDirectories("*.*");

        //This is the code that calls 
        //the getDirsFiles (calls itself recursively)
        //This is also the stopping point 
        //(End Condition) for this recursion function 
        //as it loops through until 
        //reaches the child folder and then stops.
        foreach (DirectoryInfo dir in dirs)
        {
            io.WriteLine("--------->> {0} ", dir.Name);
            getDirsFiles(dir);
        }

    }
Mike
+13  A: 

There's nothing wrong with for loops but here are some of the reasons people might prefer functional/declarative approaches like LINQ where you declare what you want rather than how you get it:-

  1. Functional approaches are potentially easier to parallelize either manually using PLINQ or by the compiler. As CPUs move to even more cores this may become more important.

  2. Functional approaches make it easier to achieve lazy evaluation in multi-step processes because you can pass the intermediate results to the next step as a simple variable which hasn't been evaluated fully yet rather than evaluating the first step entirely and then passing a collection to the next step (or without using a separate method and a yield statement to achieve the same procedurally).

  3. Functional approaches are often shorter and easier to read.

  4. Functional approaches often eliminate complex conditional bodies within for loops (e.g. if statements and 'continue' statements) because you can break the for loop down into logical steps - selecting all the elements that match, doing an operation on them, ...

Hightechrider
The list is helpful, thanks.
Lernkurve
A: 

If you abstract the for loop directly you get:

void For<T>(T initial, Func<T,bool> whilePredicate, Func<T,T> step, Action<T> action)
{
    for (T t = initial; whilePredicate(t); step(t))
    {
        action(t);
    }
}

The problem I have with this from a functional programming perspective is the void return type. It essentially means that for loops do not compose nicely with anything. So the goal is not to have a 1-1 conversion from for loop to some function, it is to think functionally and avoid doing things that do not compose. Instead of thinking of looping and acting think of the whole problem and what you are mapping from and to.

fryguybob
Oh lordy I hope no one would ever write a higher-order `For` function like that. Try `void Iter<T>(Action<T> f, IEnumerable<T> items) { foreach(item item in items) f(item); }` instead.
Juliet
That's exactly my point. `for` doesn't have an interesting type signature.
fryguybob
A: 

The question is if the loop will be mutating state or causing side effects. If so, use a foreach loop. If not, consider using LINQ or other functional constructs.

See "foreach" vs "ForEach" on Eric Lippert's Blog.

TrueWill
+9  A: 

Why are for-loops bad? Or, in what context are for-loops to avoid and why?

If your colleague has a functional programming, then he's probably already familiar with the basic reasons for avoiding for loops:

Fold / Map / Filter cover most use cases of list traversal, and lend themselves well to function composition. For-loops aren't a good pattern because they aren't composable.

Most of the time, you traverse through a list to fold (aggregate), map, or filter values in a list. These higher order functions already exist in every mainstream functional language, so you rarely see the for-loop idiom used in functional code.

Higher order functions are the bread and butter of function composition, meaning you can easily combine simple function into something more complex.

To give a non-trivial example, consider the following in an imperative language:

let x = someList;
y = []
for x' in x
    y.Add(f x')

z = []
for y' in y
    z.Add(g y')

In a functional language, we'd write map g (map f x), or we can eliminate the intermediate list using map (f . g) x. Now we can, in principle, eliminate the intermediate list from the imperative version, and that would help a little -- but not much.

The main problem with the imperative version is simply that the for-loops are implementation details. If you want change the function, you change its implementation -- and you end up modifying a lot of code.

Case in point, how would you write map g (filter f x) in imperatively? Well, since you can't reuse your original code which maps and maps, you need to write a new function which filters and maps instead. And if you have 50 ways to map and 50 ways to filter, how you need 50^50 functions, or you need to simulate the ability to pass functions as first-class parameters using the command pattern (if you've ever tried functional programming in Java, you understand what a nightmare this can be).

Back in the the functional universe, you can generalize map g (map f x) in way that lets you swap out the map with filter or fold as needed:

let apply2 a g b f x = a g (b f x)

And call it using apply2 map g filter f or apply2 map g map f or apply2 filter g filter f or whatever you need. Now you'd probably never write code like that in the real world, you'd probably simplify it using:

let mapmap g f = apply2 map g map f
let mapfilter g f = apply2 map g filter f

Higher-order functions and function composition give you a level of abstraction that you cannot get with the imperative code.

Abstracting out the implementation details of loops let's you seamlessly swap one loop for another.

Remember, for-loops are an implementation detail. If you need to change the implementation, you need to change every for-loop.

Map / fold / filter abstract away the loop. So if you want to change the implementation of your loops, you change it in those functions.

Now you might wonder why you'd want to abstract away a loop. Consider the task of mapping items from one type to another: usually, items are mapped one at a time, sequentially, and independently from all other items. Most of the time, maps like this are prime candidates for parallelization.

Unfortunately, the implementation details for sequential maps and parallel maps aren't interchangeable. If you have a ton of sequential maps all over your code, and you want swap them out for parallel maps, you have two choices: copy/paste the same parallel mapping code all over your code base, or abstract away mapping logic into two functions map and pmap. Once you're go the second route, you're already knee-deep in functional programming territory.

If you understand the purpose of function composition and abstracting away implementation details (even details as trivial as looping), you can start to appreciate just how and why functional programming is so powerful in the first place.

Juliet
Oh, and before some joker says "but aren't map, fold, and filter implemented with for-loop!". Answer: not always. They're easy to implement with recursion: `let fold f seed = function [] -> seed | x::xs -> fold f (f seed x) xs`, `let map f = [] -> [] | x::xs -> f x::map f xs`, and `let filter f = [] -> [] | x::xs -> if f x then x::filter f xs or filter f xs`.
Juliet
I knew this was a Juliet post long before I had scrolled far enough to confirm it. Well put.
Joel Mueller
Very good answer, no denying that. But re: your comment about recursion: just because some function could be implemented by using recursion just as easily as by using a `for` loop does not imply (to me, anyway) that it is therefore best--in the context of implementing such functions--to avoid `for` loops. It's begging the question, really. To me it's like saying, "You should not eat apples." "Why not?" "Because you could eat oranges, instead."
Dan Tao
A: 

A for loop can always be replaced by a recursive function that doesn't involve the use of a loop. A recursive function is a more functional stye of programming.

But if you blindly replace for loops with recursive functions, then kittens and puppies will both die by the millions, and you will be done in by a velocirapter.

OK, here's an example. But please keep in mind that I do not advocate making this change!

The for loop

for (int index = 0; index < args.Length; ++index)
    Console.WriteLine(args[index]);

Can be changed to this recursive function call

WriteValuesToTheConsole(args, 0);


static void WriteValuesToTheConsole<T>(T[] values, int startingIndex)
{
    if (startingIndex < values.Length)
    {
        Console.WriteLine(values[startingIndex]);
        WriteValuesToTheConsole<T>(values, startingIndex + 1);
    }
}

This should work just the same for most values, but it is far less clear, less effecient, and could exhaust the stack if the array is too large.

Jeffrey L Whitledge
Thanks. Can you provide a code example for such a replacement?
Lernkurve
A: 

Your colleague may be suggesting under certain circumstances where database data is involved that it is better to use an aggregate SQL function such as Average() or Sum() at query time as opposed to processing the data on the C# side within an ADO .NET application.

Otherwise for loops are highly effective when used properly, but realize that if you find yourself nesting them to three or more orders, you might need a better algorithm, such as one that involves recursion, subroutines or both. For example, a bubble sort has a O(n^2) runtime on its worst-case (reverse order) scenario, but a recursive sort algorithm is only O(n log n), which is much better.

Hopefully this helps.

  • Jim
Jim McFetridge
+3  A: 

Sometime you don't kill just one kitten.

for (int i = 0; i < kittens.Length; i++) { kittens[i].Kill(); }

Sometimes you kill them all.

uncle brad
Wrong OO. Why would a method named `Kill` inside the class `Kitten` kill itself? You should change `kittens[i].Kill()` to `God.Kill(kittens[i])`. +1 for the humor though.
missingfaktor