- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- 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
It can give one a great sense of personal satisfaction to find and clear out some rubbish like this. Finding, fixing and clearing bugs can result in a delightful sense of accomplishment.
Unfortunately I seem to have fetched up in a place where the codebase is boringly and professionally robust and clean, and bugs are rare and obscure, but when bugs do appear, they take quite some considerable time and effort to clear.
Here's to buggy and inefficient code -- it can pay for a rich and extravagant lifestyle.
Admin
Why even fix it, you'll just get blamed for all the new ones
Admin
This code is so bad, I think it's an example of malicious programming: a disgruntled employee is told to engineer software, and he commits absolute crap spitefullly.
Admin
Been there, done that, have the T-shirt.
Admin
The best bug is the one that never existed. The worst bug is the one that exists forever, causing mayhem, and becomes the macguffin in a lame Marvel movie.
Admin
The for loop does not crash if index = newIcons.Length. For example you have a list of 3 elements (=this.list.count). When you call this method with index=3, then the for-loop copies all 3 elments of the empty array (icons) into newIcons and then adds the new icon (val) to the end. (Of course the for-loop wold crash if you pass any other value than 3, but maybe they never call this method with the wrong value. That means they always insert the new icon at the end of the list) At the end this.list.count contains 4 elements, all empty except the last one.
The question is: For what purpose is a list useful that contains the latest image at the end and all other previous images are zeroed out?
Admin
My guess as to what happened here is the developer has yet to learn of System.Collections.Generic... and just knows of System.Collections. Either because they never bothered to learn .NET Framework 2.0 (where generics were introduced) or simply discovered ArrayList (which can only hold objects and not specific typed objects) and didn't bother to see if there were any better list types in the framework.
Admin
TRWTF is that Shalonda didn't replace it with a method that just does
throw new IndexOutOfRangeException()— there was probably other code that depended on that being thrown. (Only half-kidding here...)Admin
Where do you think all the new bugs are coming from? They are due to everybody else doing a work-around of the bugs with this routine and those external "fixes" still expecting the routine to return trash.
Admin
Don't try to figure out the minds of these people. I have inherited a collection of .Net applications that make heavy use of ArrayList and Object types, as well as a ton of stringly-typed things. The thing is, all of the code was written by a developer that was eleven years old when Generics were introduced.
We still use the same developer occasionally as a contractor (against my protests), and he keeps making the same mistakes. We do review his code and we always send it back with the same comments. All he does is shake his head and call us "picky" because his code works just fine. He has a sizable group of supporters within the organization that think our current push for modernization and code quality is killing the company and they are trying to get back to the good old days.
Admin
It will work only if you are inserting at the last position.
If not, if doesn't insert anything. It replaces the old icon at position i with the new icon, and leaves a null entry at the last position.
Or it would, if it didn't throw that exception...
Admin
You mean... I've failed the task successfully?? yesssssssss!
Admin
Oh noes... you better delete that comment. Or hope that a possible future employer doesn't read it...
Admin
Don't think there are no .net framework 1 projects still out there. Sometimes purple are afraid of change. They rather pay a high monthly fee to keep ancient technology running, then pay a large onetime amount to get things up to date. Behavior might change and break something else in the chain.
Admin
"because his code works just fine" - if the code works fine then you're going to need a strong argument to get it changed.
Admin
The monthly fee comes out of one budget, the rewrite the product funding comes out of another, and the first is always easier to do. Also a regular fixed cost is seen as preferable to an unknown and unguaranteed amount.
Admin
Does their version of C# not have binary trees such as red/black trees, or splay trees or even hash maps, or did they not have a database to store the data? If the software language gives you a rich array of data structures, building your own makes you an idiot. They could work much less hard and get much better results, if they trusted the language to have the relevant building block for the code. Building your own from scratch is primarily for c developers.
Admin
This looks like something I wrote in cs 101 because I didn't know what I was doing and neither did anyone else.
Admin
"Don't think there are no .net framework 1 projects still out there" - 1.0 I tended to agree... that being said I encounter one or two 1.1 based systems each year....(I am so lucky)
Admin
"It can give one a great sense of personal satisfaction to find and clear out some rubbish like this. Finding, fixing and clearing bugs can result in a delightful sense of accomplishment."
Same here. Unfortunately, in the end it doesn't really matter as eventually all code I write will be thrown away and replaced when new developers re-write it and the cycle of bugs begins again. sigh.
Admin
That is only the case when dealing with complete idiots. More commonly, there either isn't enough cost difference to justify the risk of a rewrite, or the capital and\or staff resources can be put to use in a more productive way, making enough extra to cover the costs of not rewriting.
Admin
You lot are forgetting about the risk factor involved.
If the current code "works" for any reasonable definition of the term than it is a known quanity. Even if it has bugs, those are known bugs. You have systems and procedures to work around them or legal ways to ignore them.
A rewrite on the other hand produces an unknown quantity. Sure, it might produce a perfect bug free program but you don't know that it will. Far more likely it is going to produce a new, unknown program with unknown issues and bugs in it.
And it's very hard to budget for the unknown. And sure maintaining these systems sucks for us developers. And using them sucks for the end users. But the people making the decisions are neither. They don't care if we have to gargle broken glass just as long as at the end of the day the work gets done and they get their bonus.
So from their perspective even if all other things like cost are equal it's just a huge risk with no real benifit.
Admin
Which is exactly why you never rewrite anything.
Admin
I didn't originally see that the "original" array wasn't the expected original list of images, but even if that wasn't the case there's still a major issue in the logic - it doesn't insert the new item at the given index, it replaces the image at that index (and extends the array by 1 with an empty image, again ignoring OutOfRange exceptions).
I'm annoyed that that is the bug I found, not any of the actual issues.
Admin
Some can "work", but only if used in the expected way. E.g. anything with glaring SQL injection vulnerabilities. Unfortunately, it's hard to justify the expenditure of fixing code based on the future cost of a currently-hypothetical mess that will be avoided.
Admin
How would I go about fixing it: This will obviously always throw an exception, except I think if you add an object to the end of a list, where it produces nonsense without throwing, and if you add an object to the end of an empty list, where it works. BTW if it throws, it doesn’t do anything else.
Using a system with decent tools, I refactor it renaming Insert to xxxxInsert, so all calls go to the old method, then add s correct Insert.
And then I visit every call to xxxxInsert and see what it does. I guess I find lots of try with empty catch… Each xxxxInsert I add an assert (index is at the end of the list), and an assert (empty list and index = 0). Then I run the code and every assert I figure out what happens and fix it and remove the assert.