CodeSOD: Gotta Get 'Em All |
LINQ brings functional programming and loads of syntactic sugar to .NET languages. Its a nice feature, although as James points out, it helps if your fellow developers have even the slightest clue about what theyre doing.
// some validation checking
var retrieveDocIdList = this.storedDocumentManager.GetAllForClientNotRetrieved(client.Id).Select(x => x.Id.ToString(CultureInfo.InvariantCulture)).ToList();
retrieveDocIdList.ForEach(id => {
var storedDoc = this.storedDocumentManager.Get(int.Parse(id))
// do some other stuff with the doc
});
James writes:
The code snippet is somewhat paraphrased because the actual code is poorly formatted and full of junk, but this is the main point.
It seems to be a requirement that previous developers leave weird code behind with no documentation or comments explaining what they were thinking at the time.
Well, first, poorly formatted and full of junk is our stock-in-trade, but we do appreciate the focus on the main WTF. Lets see if we can piece together what the developers were thinking.
If youve read an article here before, your eyes almost certainly will catch the x.Id.ToString and the int.Parse(id) calls. Right off the bat, you know somethings fishy. But lets walk through it.
this.storedDocumentManager.GetAllForClientNotRetrieved(client.Id) returns a list of all the documents that have not beeen loaded from the database. Now, by default, this is equivalent to a SELECT *, so instead of getting all that data, they pick off just the IDs as a string in the Select call.
Now, they have a list of IDs of documents that they dont have loaded. So now, they can take each ID, and in a ForEach call… fetch the entire document from the database.
Well, thats what it does, but what were they thinking? We may never know, but at a guess, someone knew that Select star bad, select specific fields, and then they just applied that knowledge without any further thought. The other possibility is that the team of developers wrote individual lines without talking to anyone else, and then just kinda mashed it together without understanding how it worked.
James replaced the entire thing:
foreach (var storedDoc in this.storedDocumentManager.GetAllForClientNotRetrieved(client.Id)) {
//do some other stuff with the doc
}
[Advertisement]
Incrementally adopt DevOps best practices with BuildMaster, ProGet and Otter, creating a robust, secure, scalable, and reliable DevOps toolchain.
| Комментировать | « Пред. запись — К дневнику — След. запись » | Страницы: [1] [Новые] |