CodeSOD: Skip to the Loo

Way back when Microsoft added LINQ to .NET, the real selling point was lazy evaluation. You could do something like var x = someList.Skip(3).Where((x) => x > 3).Take(5) and nothing would actually happen until you attempted to interact with the value of x. This can be especially great when interacting with a database, avoiding the round-trip until you actually need the data, and then only fetching the data which fulfills your request. If you understand what's happening, this can be pretty great.

If you understand. Which brings us to Simon, who has inherited a "particularly bad" code base. This particular system is for tracking attendance, and the pool of individuals being tracked is rather large, so someone wanted to make sure that they were processed in batches of no more than 30. This is how they accomplished that.

for (int i = 0; i < attendanceSubmission.Count; i++){ var batch = attendanceSubmission.Skip(i*30).Take(20).ToList(); if (batch.Count == 0) break; var batchNumber = String.Format("BATCH-{0}", DateTime.Now.Ticks); this.SetAttendanceBatchNumber(batchNumber, ref batch); // do stuff}

Amazingly, they managed to get everything about this wrong. They execute the loop for every item in the list. But in the loop, they Skip(i*30), and then Take(20), which is a good way to ensure that 1/3 of all your attendance submissions are lost. And, since the loop executes once for every entry, we can't rely on the for loop breaking us out, so we have to check ourselves- if the batch.Count has no items, we must have run past the end of the list and should stop processing.

I assume that, in actual use, this feature was considered "buggy" and the batch would get re-run several times, since many users would not end up in the batch. If, however, the order of the data changed, or if new rows got added over time, you could cut down the error rate by re-running batches- though I'm skeptical that everyone got processed every day.

Have fun tracking down whether it was batch 638113981750024589 or 638113982346989512 that has the missing users in it.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!