- 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
I'm a fri-st?
Admin
TRWTF is that "O(n²)" is being displayed as "O(n2)", making this loop quite less expensive.
Admin
@Hannes You meant "Am I fri-st?"
Admin
Made me think WTF for convoluated logic, with no comments. So if the interest rate for a given year is not defined, we just use the highest year or what?
Admin
I don't know yet. I'm still iterating through all comment IDs from 1 to 473649, looking up each comment on this web site until I get to the first one of today's article. Then I'll know if it's yours.
O(n^2) instead of O(n) is bad enough, but rather common. O(n^2) instead of O(1) is really WTFy.
Admin
Are C# dicts ordered in some way? It's not clear to me what Last() should do on a dictionary. Return the entry with the highest key? The last entered element?
Admin
I was iterating through all letters in "I am frist?" and this is what my "logarithm" came up with. Wonder where it got the ' from though.
Admin
Arrays? Bahaha... Are you still using Solaris or something? Arrays are soooo old, you have to use dictionaries for everything because it is totes the latest fashion in coding (takes a sip of latte)
Admin
It is not that bad - it is mapping years to interest rates, not interest rates to years.
Admin
Unless this company has been around for a hundred years or more, the algorithmic cost of this thing is of no consequence whatsoever. To take an extreme example, you can even brute-force the Traveling Salesman problem without much pain, so long as n < 10 or so. Let's not encourage premature optimization, shall we?
Having said which, this code is almost defiantly ugly. No question: it should die in a fire.
Admin
who is cow orcer? its about orcs or orcas?
Admin
No, the order of elements in a Dictionary is non-deterministic in C# - It's using a hash set under the covers, but it is IEnumerable, so .Last() will return the item at the "right side" of the underlying tree. There is an OrderedDictionary class if you actually need the best of both worlds, but its less performant.
Admin
What's the matter with you? Never seen a lady ork a cow before?
Admin
While I agree that the algorithmic cost is a non issue the fact that dictionaries are unordered is a serious problem - because you selecting the interest in an arbitrary year that came no later than the given operatingYear
Admin
So, they wanted .Item instead of .ElementAt? Sounds like TRWTF is the method names on Dictionary.
Admin
This might be the single worst piece of code ever posted on this site. The WTF to line ratio is incredible!!
Admin
If the article is correct and they where guaranteed a rate at every year they wanted yearToInterestRates[operatingYear]. If they needed to account for the possibility of years where the rate was missing, and they wanted to assume that in this case the previous rate applies then they wanted to use an OrderedDictionary
Admin
The REAL WTF is that the cost of a Dictionary lookup is not O(1). It's O(log n).
Admin
Right, that was my thought. "map interest rates to years" means:
$m->{5.5%} => [1980, 1997, 2014];
Admin
I can see it now:
New (non WTF) worker: Boss, this code is terrible, I want to make it better.
Boss: Does the code work? What is the risk of changing it? What happens if it is wrong?
New guy: Yes, it works, I don't know what the risk is. It is pretty simple, I think I can get it right.
Boss: Sorry, too much risk. It works, leave it alone.
New guy: But, but but.... (SIGH)
Another WTF code segment carved in stone.
Admin
Au contraire, dictionary lookup can be (and usually is) done via hash tables which really do have O(1) performance. I thought this was pretty close to magic when I first heard about it, but it's true and in fact pretty straightforward. You can implement dictionary lookup with O(log n) tree search, but there's not a lot of point unless you care about the order of items.
Admin
It's still not O(1). No matter how a dictionary is implemented, you still have to worry about the case where the first item you find due to your hash isn't the correct one, and then you have to search some more. This effectively makes the search O(f(n)) where f(n) is some function based on your hash table scheme. The pure hash table would be O(n) worst case. The bucket table would be closer to O(log(n)) depending on how many buckets there are and how it decides to expand the bucket count.
Admin
I just wrote a test console app which verifies that this only works so long as the dictionary is initialized in order from year zero. At least on my machine. Also, even with that initialization order, the function still returns results such that FindInterestRate(year, rates) == rates[Math.Min(year+1, YEAR_COUNT-1)]
Admin
Check the code one more time. It's not compatible with using an array. It is returning the entry value prior to the first entry where the requested
operationYear
is less thankey - 1
. An array would not allow you to skip and shuffle operationYears in order to get entirely new results.Admin
Generally all that's done during creation of the dictionary. The actual search itself should be done with a hash chosen (during the creation of the dictionary) to have no collisions, and as you correctly note this creation process will not be constant time. However, once that's done, subsequent searches will be O(1) until further modification of the dictionary.
Admin
This is designed to cope with sparse data (not all years have rates); without storing nonsense values in the dictionary (you can't do this cleanly in a C# array; unless you have an array of nullable?)
This is designed to give the interest rate in effect in a particular year; even if that year does not have an explicitly defined interest rate; in which case it gives the most recent interest rate. Be sure about the 'goals' of you code, it looks like this was written by someone that knew a tad more about the situation (and what was likely to happen down the track) than you did, when dealing with flaky and incomplete data is a requirement.
A simple array does NOT have this behaviour. Don't be so quick to judge others.
private static double FindInterestRate(int operationYear, Dictionary<int, double> yearToInterestRates) //where 0 is the first year { // Search backwards for the most recently defined year while (operationYear > 0 && !yearToInterestRates.HasKey[operationYear]) { operationYear--; }
}
Admin
Admin
If memory serves me correctly, Dictionary objects in C# can be indexed like arrays. So performance aside, wouldn't the non-WTF function be something along the lines of: private static double FindInterestRate(int operationYear, Dictionary<int, double> yearToInterestRates) //where 0 is the first year { return yearToInterestRates[operationYear]; }
Which probably isn't worth implementing as a separate function call.
So yeah. With a 100% ratio of lines-of-code to WTF, this probably at least ties for highest.
Admin
ooOOooGa: Depends -- if it truly is a 1:1 mapping from year to interest rate, then yes, don't even bother with the method call. BUT this is clearly written to answer a different question - "what is the interest rate in effect in operationYear, even if that interest rate was defined in a previous year and has not been explicitly set for the year in question."
Given that extra complexity, a method call is (in my opinion) justified; especially since the .Net CLR's JIT compiler might inline it anyway.
Admin
I see this all the time:
{v : k for k, v in some_dict.items()}.get(whatever, 'default')
Sometimes it might be a micro-optimization, but I think people genuinely don't realize that object construction isn't free. I've seen smart people do this in a loop.
If it really makes more sense to scan the dictionary, at least be lazy as possible, e.g.
next((k for k, v in some_dict.items() if k == whatever), 'default')
Admin
Yeah, array[index] is sugar for array.Item(index). Seems like they wouldn't even think to account for a missing year, because what years are ever missing, right?
Admin
If the period spans BCE to CE, there should be no year 0.
Admin
Are you saying there's some years that don't have an interest rate?
Admin
"defiantly ugly"?
Is that someone who refuses to wear make-up, brush their hair or bathe?
Admin
My company hired some outsourced programmers to write an API. On a previous project, I taught them to use Dictionaries instead of 2 arrays when mapping keys to value. On this project, they needed a collection with the index of each element (which is exactly what an array provides). Instead, the API they wrote required users to pass in a Dictionary<int, string>, with the index as the key...
Admin
They deliberately use the makeup to highlight their acne.
Admin
IMHO another just as big WTF is not returning an error or raising an exception for inputing invalid data, i.e. years for which no interest rate was recorded. Also combined with having completely inconsistent range checking. For years < 0 the interest was 0, but for years in the future, interest is the same as the last recorded year.
I have seen too many cases of code trying to halfheartedly salvage broken input and in the process just causing more damage upstream, when they really should have failed hard instead.
Admin
Well, I was joking, but the epoch in use is the company's first year of operation.
Admin
btree the dictionaries!
Admin
If OO actually worked, arrays could just implement that interface.
Admin
False. It is not my name. It is my comment, though.