- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
What are the bets that:
row(0).ToString().Equals("11229") OR
is missing a character and should instead have a 6-digit identifier?
Admin
Many devs use whitespace, files, folders, passwords... as if you had to pay for them.
Admin
As a whole, it makes my teeth itch. That one with "11229" makes my eyes itch. It just screams "error" because it doesn't match the length of the others.
Admin
I would use a hashset to find if the first row matches any of the values in the list instead of the "equal OR…" construct. Is there a better way to do it?
Admin
I also like how they've got caught up in the classic VB trap of assuming
OR
works like||
in C style languages. There's no short circuiting here so even if the index check is false or one of the firstOR
s is true, it's going to be doing thatToString()
however many timesAdmin
I like how VB's short-circuiting or operator sounds more like a threat:
OrElse
.Admin
But even if it had short circuiting this would still be na unreadable and unmaintainable mess. Why not just have an array of values and use foreach on it? If you know to use string.equals you know to use bloody foreach.
Admin
How about the redundant call to ToString on each and every test?
Admin
This is probably something that grew over time. Originally there were only 1 or 2 entries, so the OR made sense, but every now and again another one got added, until it grew to this monstrosity.
Admin
Clearly, "30760" is the favorite: entered not just twice like some, but thrice!
$ sort < qq1 | wc -l 128 $ sort -u < qq1 | wc -l 117 $ diff -y --suppress-common-lines -W 20 qq[12] 30005 < 30603 < 30702 < 30705 < 30760 < 30760 < 30763 < 30770 < 30818 < 30853 < 30993 < $
Admin
It seems likely that there's a database behind this beast, so I'd create a table and let the DBM worry about caching.+66
Admin
The general shape of the conditional is
index <>0 AND {wall-of-ORs} AND RecordDate more than 14 days ago
You'd hope the optimizer would reorder those to avoid even starting down evaluating the wall-of-ORs unless both the other conditionals are true.But it would sure be nice if the dev had thought of that him/herself and put that date check up top as the second item
index <>0 AND RecordDate more than 14 days ago AND {wall-of-ORs}
And had used the short-circuiting versions of both
And
andOr
:AndAlso
andOrElse
. Maybe the current VB.Net compiler will do the analysis to determine whether there are side effects to substituting the short-circuiting operators, but I tend to doubt it.Stupid from one end to the other.
Admin
No, the compiler doesn't do the short-circuiting for you, as that would be great in certain circumstances but would cause huge issues in others where the OR and alternative ORELSE syntax have been used properly.
There's loads of ways this could have been done in VB.Net, and they picked the very worst.
Admin
$ sort < qq1 | wc -l 128 $ sort -u < qq1 | wc -l 117 $ diff -y --suppress-common-lines -W 20 qq[12] 30005 < 30603 < 30702 < 30705 < 30760 < 30760 < 30763 < 30770 < 30818 < 30853 < 30993 < $
Maybe this is a poor man's "exclusive" OR?
Addendum 2020-10-07 09:27: EDIT: Sorry about messing up the block quote
Admin
Put the items in a list (static constant), then IndexOf.....
Admin
Nah, I'd have written an excel formula to generate all of these automagickally.
Admin
Lambda, the ultimate job security.
(In reference to the Lambda the Ultimate series of papers: https://web.archive.org/web/20180607112856/http://library.readscheme.org/page1.html )
Admin
Could it be that the customer object is 011229 and the leading zero isn't being added by ToString ?
Admin
I like how the DateTime.Today.AddDays(-14) is built into this filter function, so it will be called every single time.
Admin
No short-circuit is a great protection against timing attacks. VB should be the language of choice for the most secure systems.
Admin
(Puts hand up tentatively ...)
"Please Sir, how expensive is a ToString?"
Admin
The VB.Net compiler can be set to issue warnings if you use AND or OR in a conditional and will remind you that you might want to use AndAlso or OrElse instead, but it does not do the change for you.
Admin
That's right. You could more easily do an integer comparison in the lambda, but Integer.ToString is not that expensive, but each one is putting a new string on the heap, in fact the same string, as you are converting the same integer each time. I doubt the optimizer will say, "Hey I know that already. I can just use that string over there."
Admin
Why do some many assume that row(0) contains an integer? The first ToString compares to "DIV10106", I have never seen that kind integer. It might be that this is the result of a SQL join or whatever.
Admin
I've use C# and VB.NET, and the main thing that bothers me about VB.NET is the lamba syntax of object.Where(Function (x) x...
Admin
Mad props for that.
Admin
Not that I would suggest ingesting VB.Bleach, but I'd be quite happy with separating the lambda out. (At which point it's arguably not a lambda, but still.) Something like:
Not tested, but should be close to working. The advantage here (which also applies to C#) is that you can reuse the lambda in cases where (for example) you want FirstOrDefault applied to a list of value types with no default.
It's almost elegant, if you look at it with swivelled eyes.
Admin
That's the customer-reference version of FileNotFound.
Admin
That's TRWTF. TRRWTF is not using a set for performance and instead making many expensive string comparisons.
Admin
@Mathieu CLAVEL: Given that we have a row object (possibly pulled from a database or other table structure), how about adding a column, with a name that gives context, that is set to true for the given values, and then simplify everything to if([...] AND row(N-1))?
This also feels like it started off as a short list that got items added over time. (Note how the values increment most of the time as you scroll down the list.) This could explain why SaveTime McTimeSaver opted for a lambda instead of a DB modification, as it is probably faster to implement for up to 5 records.
Admin
Maybe, but even then, it stands out as the only one that's shorter like that.
Admin
Compilers can be evil/helpful enough to optimize even though there's no short circuiting. If they can know the end result is actually the same (function isn't going to throw), they may decide to just optimize it into short circuiting anyway.
Admin
Makes sense. However, a hashset makes it even easier to extend the functionality by copy-pasting code, whereas the proper solution is to store the matching values in the/a database. An alternative data structure should rather have the opposite effect (making it less easy to extend, thus forcing subsequent developers into finding a proper solution).
With that in mind, I would define a static radix tree and resolve it inline by a recursive lambda (by using a lambda that takes another lambda as one of its arguments).
Admin
@Steve Ref
I wasn't suggesting the compiler simply substitute
OrElse
forOr
orAndAlso
forAnd
willy nilly. That's clearly a very bad idea.An optimizing compiler can in principle make those substitutions and preserve correctness when (and only when) it can reason accurately about the side effects of the clauses on either side of the conjunction. And whenever it can't so reason, it leaves the operators as written. Likewise a skilled optimizing compiler can re-order the sequence of clauses as well to improve performance, but only when it can determine any dependencies between the side effects of one clause and the evaluation of another clause.
After all, if a skilled programmer is using the short-circuiting operators correctly, and/or sequencing his clauses for maximum performance, it's only because they have correctly analysed that suppressing the right side clauses or ordering the clauses as they have is consequence-free.
All of this is bog-standard modern compiler design. My question was whether any version of the VB.Net compiler does this. The answer may well be "no" as you say. My trawl through MSFT's docs was very uninformative.
Admin
Microsoft's Roslyn compiler is open source: https://github.com/dotnet/roslyn
Admin
I'm not sure there's a good case for trying to optimize non-short-circuiting into short-circuiting. In trivial cases, there wouldn't necessarily be a performance impact (since the CPU may well be able to execute both sides of the conditional simultaneously in different execution units). In non-trivial cases, the optimizer is unlikely to be able to safely perform the conversion, because there is likely to be a function call involved. Indeed, it wouldn't be safe to do that optimization in this case because of the call to ToString.
Admin
People, people. Forget all this talk about the structure of the code or calling speed thanks to a million toStrings and all that. This code is using a numeric format to store a non numeric value, that being the ID code. What is going to happen if what ever system is in charge of handing out codes suddenly decides to hand out one starting with a 0?
Admin
Doesn't need a lambda, this is a basic map .
Admin
First look - oh, sure, this looks like a standard result of being paid per LoC. Except you added the line breaks and the original was a single line. What the...
Admin
Hi, I've discovered that the mysterious "DIV10106" is also a customer object reference. It's just different, for no apparent reason.
Admin
Because sometimes people on this forum like to be TRWTF. Based on from Eric two weeks later, it's not an integer.
That doesn't stop people. I've been on a call with a half dozen people making speculative comments about optimizations that could be done due to Employee Number being a number while the shared screen showed a list of sample data in which roughly half of the Employee Number values were obvious non-numbers such as 0001234, HRM4321, and 666SOA.
Never mind the fact that the database slowness being discussed was on a completely different column, and the people talking about how Employee Number could be optimized were from the attribute that perpetrated the app that was trying to substring search on data in a large BLOB column.
For what it's worth, they did have other columns they could've selected on first, but they opted to save time and just go straight for the one criterion that would unambiguously get exactly the entries they wanted and nothing else. Eventually, I managed to talk them into going to that hassle. Of course that could never work, but fortunately, the database slowness issues just happened to go away around the same time they did that.