Step into the confessional. Now's your time to come clean.
Don't tell us about code you inherited or from some co-worker. This is about your personal growth as a programmer and as a person.
I get remined of a quote: Smart programmers create unmaintainable code.
I've had the first hand experience of this, when a ORM written in C ( back in 1997 ), which used elegant #define tricks, was replaced by the C-pre-processor output, by my successor.
Although it was one of the most beautiful code, the lack of documentation made it one of the worst code I'd written.
/////////////////////////////////////////////////////
// WARNING! PLACEHOLDER FUNCTION!
// This function to sort the data is hugely inefficent,
// Use only for initial testing on small data sets
// MUST BE REPLACED BEFORE FULL DATA SET IS USED!
A poorly coded bubble sort on a custom data type works great for testing during the initial proof-of-concept phase to show the core design works, but when a pointy haired boss decides against your advice the app is ready to show off with prodution data and the number of elements increases from a few dozen to a few million ... not so good.
This was more a lesson in dealing with management than in coding, since I knew all along exactly what would happen if the code didn't get replaced. (Eventually it was rewritten using quicksort, which reduced the sort time from several days to a few minutes.)
Don't know about "worst" but this (well, similar) was in production code that came up in a code review :) The if was supposed to check for some special condition to avoid bugs.. hmm..
if (someVariable == someVariable) {
// do something
}
string @string = @"new @string()".ToString();
@string = @string == @"string" ? @string : @"Hello World!";
Why was it so bad: Obfuscated, redundant… and useless.
(It made for a nice background though.)
I don't have the specific code, because this was a LONG LONG time ago but my very first programming job out of university was using ColdFusion and SQL Server. I didn't really know what I was doing, as I had never really used a "real" database before. So I didn't know some pretty rudimentary things. Like JOINS.
I made this rather large and rather complex web application for a rather large and rather complex organisation and all the way through the codebase was this startlingly awesome pattern:
- SELECT * FROM table
- loop through each record
- SELECT * FROM another_table WHERE key = table.key
- loop through each record
- SELECT * FROM yet_another_table WHERE key = another_table.key
- loop through each record
- probably some complex IF-THEN-ELSE-ELSEIF-UNLESS-ARGH condition
- etc
- etc
Any change to the database would cascade through loops and conditions nested in ever-increasing levels of insanity and ignorance.
I still remember the awe I felt at being to simple write a single query that joined all the data together.
I once made an on-screen keyboard that checked if the user was pressing a key like this:
-- A
if stylusInBox( 54, 101, 68, 115 ) then
insertCharacter( "A", 54, 101 )
end
-- B
if stylusInBox( 121, 116, 135, 130 ) then
insertCharacter( "B", 121, 116 )
end
-- C
if stylusInBox( 91, 116, 105, 130 ) then
insertCharacter( "C", 91, 116 )
end
And the same repetitive code for every letter and number.
Also, this was pretty bad:
// Even though the user id is passed to the function, only call this with the user currently logged in.
function update_users_online($user) {
Worst code you could ever find is usually Research Code.
Pressing deadlines, general lack of supervision and the notion that no one is going to see or use this except me produced some real wonders.
<?php
$iShower = $_GET['clean'];
if ($iShower) {
echo getdate();
}
?>
Oh, and suffering a severe lack of sleep, the other day I stared at:
if ($value = $some_value) {
// do stuff
}
For what must have been at least 30 minutes.
I once created a singleton object that permeated the entire code base. A new requirement came in that required slightly different forms of processing (which will be done by that singleton object!) depending on the source of the input. To address that requirement I marked the singleton with a [ThreadStatic] attribute and had the various sources running off separate threads, so that each thread would have its own singleton reference (which knew how to process the input differently)!
It's the not-so-single singleton! :( mea culpa...
Not entirely compliant with the request, as I didn't initally create it and I didn't do it willingly, but I did contribute to it:
At my first Real Programming Job, we had a client who wanted their existing COBOL app ported to Visual Basic. Not a VBish rewrite, but a direct port, preserving the appearance and the behaviour of the original.
Imagine a window filled with two-hundred-and-some text entry boxes, none of which do anything at all except let you type into them, except for the last one at the end of the page, which has an OnExit handler that's three miles long and validates/processes all the data from the entire screen...
I dimly recall a Crystal Report containing a SQL statement sooooo large that it had to be edited in a text editor and then have all unneeded whitespace stripped so that it would fit within the limits of the Crystal text area (shudder)
I once inherited an stored procedure that is 40 thousand lines long.
That is a single stored procedure.
The file is about 1.5 MB.
I'm still looking for the person who wrote that to get his confession.
Ok, now for my confession.
About a decade ago, I wrote a very buggy function called InitFirstChar (I don't archive copies of source code from previous work/companies so I can't show it here). This is part of a barcode scanning library. That piece of code is so complicated and buggy that whenever we got a problem - that is immediately the very first suspect.
That function got so famous that even on our other projects - projects that is entirely unrelated to that barcode scanning library - the team still mentions InitFirstChar whenever were looking for a defect.
Every Swing Control I've ever written has been an abomination stuffed with FlowLayouts nested in BoxLayouts with margins that were determined by trial and error.
I eventually gave up on Swing and accepted that I shouldn't code GUIs
if(password='password')
{
return true;
}else{
return false;
}
I was simply stubbing out an authentication function for later development... but... :-)
12 years ago, before I knew anything about databases or SQL, I would write the most horrible iterative VBA code to work on data in MS Access. Typically, I'd SELECT the entire table into a recordset and loop through the whole thing, operate on the data via the recordset methods, one row at a time, and batch-update the entire thing at the end. Of course, all of my business logic went right in the loop.
A "SQL in 10 Minutes" book, or just someone to ask questions of, could have saved me sooo much trouble. When I finally learned about UPDATE, INSERT, JOINs and all the rest, it was like discovering fire.
Unfortunately, this was so long ago I no longer have the code. However, there is a particular instance that stands out in a rather spectacular fashion: It was an Win32 App I wrote using MFC in C++. In summary: there was a lot of incredibly complex Windows GDI code. When the application was ran it didn't merely crash, it killed the display driver (as observed by the cool unintentional visual effects and the frozen mouse cursor). At least I can have some measure of pride in writing an application that brings down an entire 32-bit Operating System in a matter of milliseconds without bothering with the dull "Blue Screen of Death".
In a language from which COBOL was generated we had to write things like:
if 1 <> 1 then
... read database tables to memory
end if
because this way it had looked like if the DB data is in memory, so we were able to use the table and field names. I hated it.
On my first programming job, I spent a whole day to write a function to add a day to a given date. Taking into account leap-years etc.
The language back then was Progress 4GL, that understands a syntax like:
nextDate = oldDate + 1
Written not a few weeks ago, trying to tame an unruly and awful old schema into something that makes a bit more sense. This is just a part of the SELECT clause:
ISNULL(CONVERT(varchar(50),CASE
WHEN HouseName is not null or HouseNumber is not null THEN
CASE
WHEN COALESCE(LEN(HouseName),0) + COALESCE(LEN(HouseNumber),0) + 3 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 THEN
--We can merge hn,hn + one address onto line 1
COALESCE(HouseName + ', ','') + COALESCE(HouseNumber + ' ','') + COALESCE(AddressLine1,AddressLine2,AddressLine3) collate Latin1_General_CI_AS
WHEN HouseName is not null then
HouseName collate Latin1_General_CI_AS
WHEN LEN(HouseNumber) + 1 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 then
HouseNumber + ' ' + COALESCE(AddressLine1,AddressLine2,AddressLine3) collate Latin1_General_CI_AS
ELSE
HouseNumber collate Latin1_General_CI_AS --Can't get anything else to share space on line 1
END
WHEN AddressLine1 is not null THEN
AddressLine1 collate Latin1_General_CI_AS
WHEN AddressLine2 is not null THEN
AddressLine2 collate Latin1_General_CI_AS
ELSE
AddressLine3 collate Latin1_General_CI_AS
END),''),
CONVERT(varchar(35),CASE
WHEN HouseName is not null or HouseNumber is not null THEN
CASE
WHEN COALESCE(LEN(HouseName),0) + COALESCE(LEN(HouseNumber),0) + 3 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 THEN
--hn,hn + one address went on line 1, find the next address
CASE
WHEN AddressLine1 is not null then COALESCE(AddressLine2,AddressLine3) collate Latin1_General_CI_AS
WHEN AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
WHEN HouseName is not null and HouseNumber is not null then
--HouseName went on line 1, let's try to get the house number in
CASE
WHEN LEN(HouseNumber) + 1 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 then
HouseNumber + ' ' + COALESCE(AddressLine1,AddressLine2,AddressLine3) collate Latin1_General_CI_AS
ELSE
HouseNumber collate Latin1_General_CI_AS --Can't get anything else to share space on line 2
END
WHEN LEN(HouseNumber) + 1 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 then
--HouseNumber + ' ' + COALESCE(AddressLine1,AddressLine2,AddressLine3) went on line 1, let's find the next address line
CASE
WHEN AddressLine1 is not null then COALESCE(AddressLine2,AddressLine3) collate Latin1_General_CI_AS
WHEN AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
ELSE
COALESCE(AddressLine1,AddressLine2,AddressLine3) collate Latin1_General_CI_AS
END
WHEN AddressLine1 is not null THEN
COALESCE(AddressLine2,AddressLine3) collate Latin1_General_CI_AS
ELSE
AddressLine3 collate Latin1_General_CI_AS
END),
CONVERT(varchar(35),CASE
WHEN HouseName is not null or HouseNumber is not null THEN
CASE
WHEN COALESCE(LEN(HouseName),0) + COALESCE(LEN(HouseNumber),0) + 3 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 THEN
--hn,hn + one address went on line 1, find the next address
CASE
WHEN AddressLine1 is not null and AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
WHEN HouseName is not null and HouseNumber is not null then
--HouseName went on line 1, let's try to get the house number in
CASE
WHEN LEN(HouseNumber) + 1 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 then
--HouseNumber + ' ' + COALESCE(AddressLine1,AddressLine2,AddressLine3) went on line 2, let's get the remaining address info
CASE
WHEN AddressLine1 is not null then COALESCE(AddressLine2,AddressLine3) collate Latin1_General_CI_AS
WHEN AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
ELSE
--HouseNumber --Can't get anything else to share space on line 2
COALESCE(AddressLine1,AddressLine2,AddressLine3) collate Latin1_General_CI_AS
END
WHEN LEN(HouseNumber) + 1 + LEN(COALESCE(AddressLine1,AddressLine2,AddressLine3)) <= 50 then
--HouseNumber + ' ' + COALESCE(AddressLine1,AddressLine2,AddressLine3) went on line 1, let's find the next address line
CASE
WHEN AddressLine1 is not null and AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
ELSE
CASE
WHEN AddressLine1 is not null then COALESCE(AddressLine2,AddressLine3) collate Latin1_General_CI_AS
WHEN AddressLine2 is not null then AddressLine3 collate Latin1_General_CI_AS
END
END
ELSE
AddressLine3 collate Latin1_General_CI_AS
END),
1969, Lancaster PA, summer job: building an accounts payable application to run on an NCR-500. I had no idea how negative numbers were represented on this machine, so in my first iteration, I guessed "nines complement". Reading a few cards keypunched with credits quickly revealed my guess to be incorrect; they were all off by a penny. "No problem", said the company's comptroller, who knew nothing about computers, "we'll just tell the keypunch operators to add a penny to each credit they enter". To avoid confusing the keypunch operators, every other application I wrote that summer -- accounts receivable, general ledger, payroll -- used the erroneous nines complement representation for negative numbers.
I went back to visit many years later, and they were still adding a penny when keypunching a credit -- but no one remembered why.
I wrote a Perl kludge which was basically a lot of lines of code wrapped around a find command call using backticks. It was more shell than perl.
I wrote some research code for a massively parallel application that ran for about 2 months, after which I realized that a single if statement was wrong :(
I think this has to be a serious contender for the worst code I've put together. Names of objects have been changed to protect the innocent.
Class myClass = someoneElsesObject.getClass();
Field privateDoNotTouchField = myClass.getDeclaredField("doNotTouch");
privateDoNotTouchField.setAccessible(true);
Object myValue = privateDoNotTouchField.get(someoneElsesObject);
privateDoNotTouchField.setAccessible(false);
MyRealObject mro = (MyRealObject)myValue;
Breaks encapsulation to access a private field? Check. Depends on the internal implementation of a library I don't control? Check. (someoneElsesObject really was someone else's) Shatters like glass if someone later puts in a SecurityManager? Check. Done for convenience alone? Check.
Edit: I should also mention that this cut right through a layer of indirection. SomeoneElsesObject was actually an interface: the doNotTouch field was specific to the class that appeared at runtime in my debugger.
Many moons ago I was doing Excel macro programming in VBA. I didn't know anything about creating functions for repetitive code, or just generally "being efficient." So I was copying blocks of code over and over, until I finally hit the limit of the number of lines allowed in a VBA module.
I have wished many times that I could spend one week at that position and rewrite all of my old macros. I hope my name isn't in the code anywhere... :-)
Did a menu tree in php that read from an SQL by an id. Id's were int but if someone put a letter in the $_GET statment it would loop in such a way that it used all the memory resource of the server, in the end crashing it, and all the 200 other webs hosted on it.
' Grab Unique Check Numbers from all numbers
Dim UniqueCheckNum(1000)
Dim TotalUniqueChecks
TotalUniqueChecks = 0
for i = 0 to TotalUniqueChecks
for j = 1 to TotalElem
if NOT ChapCheck(j) = UniqueCheckNum(i) then
UniqueCheckNum(TotalUniqueChecks) = ChapCheck(j)
TotalUniqueChecks = TotalUniqueChecks + 1
end if
next
next
Written by me in the first project I ever did using VBScript.... It may look odd at first, but when you study it, you'll really wonder HTF it worked (it does work).
I only wish I had the code to share... our shop used to be heavy on Lotus Domino. I had to write a "cutting edge" DHTML UI application for a new major application's readiness scorecard. There were roughly 3000 elements needing to be displayed in a collapsable hierarchy.
I stand by the notion that it was done out of necessity, rather than naivety. I KNEW it was wrong from the start. The "solution":
Use a Notes database to extract 40-50k records, mash it around and using Domino syntax and LotusScript, produce code that dynamically produced HTML & JavaScript that would be sent to the browser. The final pages sent over the wire were very JS heavy (before we had fancy JS frameworks mind you), and it was a miracle that it even worked. It actually looked quite nice to the end user, but I would NEVER want to step into that code again.
In typical fashion, since I'm still at my current employer - though in a different division doing architecture and management activities - I get emails when it's down, and the occasional question.
It took me forever to figure out why this would always run even if the condition was false:
if (some_bool);
{
//do stuff
}
Damn C-like languages and their semicolon shenanigans. :-)
Obviously you folks don't know there's a whole art in writing obfuscated code!
I once created an XSLT (ok, this alone should be enough for a dozen upvotes!) that generated shell script that invoked another XSLT N times with different parameters. I was working around XSLT 1.0's limitation of "1 output file per transformation".
Some VB6 code that I've inherited:
If some_condition Then
send_email()
else
'An error occurred!
End If
I love it when comments substitute for proper error handling.
The worst code I have written was any code that seemed to fix the problem without my full understanding of why. I believe that Dave Thomas and Andy Hunt call this "Programming by Coincidence". My sincere apologies to anyone who has had to work with those fixes of mine, because they probably broke later.
Not quite sure how or why, but I ended up writing:
public string GetUsername (string userName)
{
User user = DbLookup.GetUser(userName);
return user.Username;
}
Really have no idea why. Not sure what I was smoking...
When I was a kid, I LOVED a TI99/4a game "Tunnels of Doom". I really wanted to write a game like that. I didn't know anything about even the most simple of data structures (arrays, etc), so I did not know how to make a maze. I remember considering hard-coding each cell in a maze (ie: if (cell=1520) then allowup = 1 etc) I ended up writing a game called "The Endless Corridor", which was, you guessed it... an infinitely long hallway. Your character would take a step east, and then fight a monster. Take another step, fight another monster.
There were no graphics calls, but you could redefine ASCII characters using a 8x8 (or something) pixel map. I had great fun in drawing monsters using graph paper and translating them into hex. Players and monsters were 2x2 character blocks. I had stacks of graphing pads full of this stuff. In defense, this all took place around age 10.
A couple years later, I wrote a game called "Castle Zadrexiak" (something like that) which was inspired by Wizardry. (Fake-3D dungeon crawler where moving was either forward, back, or a 90deg turn left or right) It at least was 2D array based, but the line drawing code was horrible. The view was defined by which walls needed to be drawn. Variables representing which wall was to be drawn were "a" for the wall to the left, "b" for a wall in front of you, and "c" for a wall to the right. If there was nothing blocking your view to the left, then the walls in that area were prefixed with "a", so "ab" would be something like a front facing wall in the square to your left. This went sometimes 3 map squares deep, so variables ended up "aabc", "abcca", etc. There were several hundred lines of code checking map square contents and deciding which walls to draw. I totally lost any ability to debug it about 1 day after finishing it.
Oh yeah, the monster balance was all off... and after about 10 minutes you'd almost always get your butt kicked by a vampire or something.
I love these memories of early programming, but the code was absolutely awful. It's too bad I've lost the code from those times.
Each time something like this happened, I knew I had to learn something. These problems absolutely drove my progress as a programmer. Back then, everyone around me was totally nontechnical. There was nobody to help me. There were no books, really. (Just the reference manuals that came with GWBasic, etc) It really taught self reliance and problem solving.
I remember the need to figure out how to save player stats so you didn't start from scratch every time being a particularly hard one at the time. I think that was about the only time I ran to my parents to explain and show off a solved programming problem... which considering they could barely turn one on had to be really hilarious. "That's nice, dear." "... but with RECORD syntax my variables will come back!" ... "That's nice, dear."
It was some sort of searching or sorting code involving a large array. This was a long time ago and I don't remember the details. It was after I had a math degree and before I had a CS degree. Anyway, I remember that I implemented it in a horribly inefficient way. A year or two later, I was a CS major and taking an algorithms class. I then realized what my mistake was (I didn't even know it was a mistake at the time and there was a more efficient way of doing it).
That was when I realized the importance of taking at least an introductory data structures and algorithms course if you want to be a programmer. Before that I had been a math snob and thought studying math would make you a better programmer than studying CS would. I still think studying math will make you a better programmer, but it's not sufficient in itself.
Several years ago I had to create a way to order callee information in a database according to 15 custom fields and 5 permanent fields. Custom fields could be of any basic type (integer, string, date, bit).
Instead of building a query generator in the application, I decided to build a HUGE function in SQL and, obviously, the guys at my company were far from happy when I left for another job and they had to change / debug it.
Several of them still do not talk to me, even though it has been 7 years since this happened.
I once wrote:
if (strSomething != "somevalue" || strSomething != "othervalue") {
//do something (this allways happens)
} else {
//do something else (this never happens)
}
.. until I realized that the statement was allways true.
Not exactly the worst code I've written but definitely a mistake I learned from.
So, when I was a child, at age 9 or 10, I wrote the "Tron" game in Basic. You know, the famous lightcycles scene from the film. I did, as surely many others have, copy the idea and make the game. I wrote the whole thing in one single afternoon, and then I typed "run". That was my methodology back then: write the whole program and type run. No unit testing or scrum meetings or so.
Anyway, the program didn't work. At all. It was not a minor bug, there was just something very wrong that prevented the game to run. I reread the code again, checked every line, but could not find what the mistake was.
A couple of years later, I decided to try it again. I had lost the original source. I could find the first printed page, but everything else was lost. So I rewrote it from scratch. Not from my memory but starting the design again. When it was finished, I typed "run". And again, it did not work. Crash. I could not believe it. What was wrong? And how come it was failing exactly the same way the other time did?
This second time, fortunately, I could find the bug through some primitive debugging. I was storing the number of players, that could only be 1 (single player against the computer) or 2 (two players sharing keyboard) in a variable called "j". And, obviously, "j" was also used everywhere in nested loops, overwriting itself. The program crashed somewhere when j as in "number of players" was more than 2.
The amazing thing is that I had commited the same mistake twice.
The lesson I learned was to name my variables more carefully in the future, to prevent duplication.
(Note: "j" is the first letter of "jugadores", which means "players" in spanish. Thus the variable's name)
Unnecessary code = bad code.
My worst code = My most unnecessary code.
It was many years ago. I didn't know regular expressions.
I wrote about 30,000 lines of sophisticated parsing code. It was robust code... but a half dozen regexes would have done the trick, and been a thousand times easier to maintain.
Regexes can do almost anything. Learn them.
Not exactly bad code I wrote, but I recently copy-pasted a set of about 10 lines of code (not written by me) to a dozen other places in the code. (I only did it because that was already done everywhere in the code by the previous coder and I was too lazy to sort it out at the time.)
Apart from the fact the lines should have been in their own function (they are now!) the most stupid thing was that amongst these 10 lines there were 2-3 lines of commented-out code, and at least 1 line declaring an unused variable! Oh, and the variables all has "2" at the end (e.g. lSqlQuery2).
A few weeks ago I found an old PHP script with some code that did the following:
Obviously a bit of overkill that should have just been a proper database query selecting specific fields, ordering and limiting, with one loop displaying the output (or maybe two loops if you're using an MVC variant).
I'll just point you to my favorite site, which is riddled with examples of the crap code I've had to deal with.
I direct you to The Daily WTF for all the awful code examples you could ever want.
Some of the worst code I have had to deal with turned out being my own. I worked a job many years ago when I first started programming. I left, matured and gained a lot of experience. Returned some years later and had to manage my own mess. The first day on the job I said, "I hate myself several years ago."
My favorite came with an application that I inherited.
It's a VB.Net app and instead of fetching the selectedvalue from a dropdown in a one-liner like this:
dim value as string = dropdown.selectedValue
The original developer looped through the list
Dim value as string
For i As Integer = 0 To Me.dropdown.Items.Count - 1
If dropdown.Items(i).Selected Then
value= dropdown.Items(i).Value
End If
Next
Now this is a working solution and it solves the problem. But when you have 20+ dropdowns on a screen, this becomes a nightmare to maintain.
I won't post code examples, but the worst kind of code I've dealt with is when you have to add a new feature to a poorly documented, mission-critical system that nobody really understands.
You have to really dig to even find the relevant code, and when you do, it is a terrible database schema represented by obtuse classes, layered in 300-line functions, all of which do seven things. Unit tests? Don't make me laugh. Touching it is scary; deploying changes is an exercise in hair-raising thrills. Work like that is genuinely depressing.
I inherited some assembly code. It was built in different ways to do different things, using preprocessor statements, so for example:
if IS_THIS_NETWORK_TYPE
if IS_CLIENT
include <something_this_client.asm>
else
include <something_this_server.asm>
endif
else
if IS_CLIENT
include <something_that_client.asm>
else
include <something_that_server.asm>
endif
endif
You'd get these conditional includes in the middle of a routine, sometimes more than once, so ...
proc my_routine
...statements...
...statements...
...statements...
if etc
include one of 4 different source files !
endif
...statements...
...statements...
...statements...
if etc
include one of 4 more different source files !
endif
...statements...
...statements...
...statements...
endp
Furthermore:
Here is an example of a coworker of a friend of mine:
It is SQL has cursors, several update insert statements, 11 tables are being created
it is too long to post here but take a look here: http://forum.lessthandot.com/viewtopic.php?f=17&t=140
If you are an Admin then you are assigned the variable IsADMIN (with a value of 1 I think although the value was never used). If you are NOT and Admin ... no variable.
<CFIF NOT IsDefined("IsADMIN")>
... do some non-admin stuff ... I think?!?!
</CFIF>
I have to deal with a legacy Classic ASP (VBScript) codebase on a daily basis, and I've seem some truly terrible code. I mean no indentation, cryptic variable names, duplicate logic that's 99% the same between the If/Else clauses, For loops inside of other for loops inside of nested If statements, with the Else statement being the exact same loop... On Error Resume Next littered on every page and, because it's there, the programmer just wrote code without rhyme nor reason so if you remove On Error Resume Next, the page won't even load.
Here's a snippet (all indentation is left as-is):
ctr=0
if ubound(linary)>35 then
'loop through the lines to get at the data
for each a in linary
ln=cstr(linary(ctr))
if left(ln,1)="D" then
psku= trim(mid(ln,2,15))
puom= trim(mid(ln,17,2))
prest=mid(ln,19,800)
pstr=""
pstrt=1
pstp=8
for b=1 to 100 step 1
pstr=pstr&mid(prest,pstrt,pstp)&","
pstrt=pstrt+8
next
pstr=left(pstr,len(pstr)-1)
myFile.writeLine psku&","&puom&","&pstr
end if
ctr=ctr+1
next
Here's another one:
if pIdCustomer=0 then
pPriceColumn="retail"
else
mySQL="select priceColumn from customers where idCustomer='" & pIdCustomer & "'"
call getFromDatabase(mySQL, rsTemp, "ViewItem_1")
pPriceColumn=rstemp("pricecolumn")
end if
if pIdCustomerType=4 or pIdCustomerType=5 or pIdCustomer=17873 or pIdCustomer=21385 then
mySQL="Select accessLevel,groupId from customers where idcustomer="&pidCustomer
call getfromDatabase(mySQL, rstemp, "comersus_customerUtilitiesMenu.asp")
end if
if not rstemp.eof then
paccessLevel=rsTemp("accessLevel")
pIdGroup=rsTemp("groupId")
else
paccessLevel=1
pIdGroup=0
end if
And the whole application is written like this, with over 5,000 ASP files.
Oh, I forgot - there's also a file of javascript functions called "jfunctions.asp" (Yes, it's an ASP file that's nothing but Javascript) that uses some weird IE-only Remote Scripting to mimic Ajax. Here's a sample from that file (as if I haven't blinded you enough). It's not so much that the code is BAD, just that it's a nightmare of spaghetti and nearly impossible to maintain; it's like reading hieroglyphics:
function movecalc(row){
var row = row
//alert(document.getElementById('wght'+row).value);
var prct = 0;
var qcst = parseFloat(document.getElementById("pQuoteCost"+row).value);
var cst = document.getElementById('pCost'+row).value;
var pqty = document.getElementById('pqty'+row).value;
var pprc = document.getElementById('pQuotePrice'+row).value;
var extqprc = 0;
var npprc = parseFloat(pprc);
//alert("New Price: "+npprc+" - QuoteCost: "+qcst+" - Qty: "+pqty+" - QuotePrice: "+pprc+" - Cost: "+cst);
document.getElementById('pQuotePrice'+row).value = npprc.toFixed(2);
if(pprc!=0){
//alert(pprc);
if (qcst>0){
prct = ((pprc - qcst)/pprc)*100;
} else{
prct = ((pprc - cst)/pprc)*100;
}
extqprc = parseFloat(pqty * pprc);
}
else{
var cprc = document.getElementById('pCustPrice'+row).value
//alert(cprc + " pprc=0");
if (qcst>0){
prct = ((cprc - qcst)/cprc)*100;
} else{
prct = (-(cprc - cst)/cprc)*100;
}
extqprc = parseFloat(pqty * cprc);
}
document.getElementById('pct'+row).value = prct.toFixed(2);
extqprc = parseFloat(extqprc);
document.getElementById('pQuoteExtend'+row).value = extqprc.toFixed(2);
//alert("fired");
allTotals();
}
I encountered this free MD5 code. The VB code for getting an md5 of a file:
Private ByteBuffer(63) As Byte
...
Do While Not EOF(1)
Get #1, , ByteBuffer
If Loc(1) < LOF(1) Then
ByteCounter = ByteCounter + 64
MD5Transform ByteBuffer
End If
Loop
Yeah, reading a file 64 bytes at a time is not a good idea. SLOW. And hardcoding a file handle is not so smart either.
Oh good god, you don't even want to know.
Typical would be this:
Public Shared Function CompressEncrypt(ByVal e As String) As String
Dim _ces As String
_ces = e
_ces = _ces.Replace("00", "g")
_ces = _ces.Replace("11", "h")
_ces = _ces.Replace("22", "i")
_ces = _ces.Replace("33", "j")
_ces = _ces.Replace("44", "k")
_ces = _ces.Replace("55", "l")
_ces = _ces.Replace("66", "m")
_ces = _ces.Replace("77", "n")
_ces = _ces.Replace("88", "o")
_ces = _ces.Replace("99", "p")
_ces = _ces.Replace("aa", "q")
_ces = _ces.Replace("bb", "r")
_ces = _ces.Replace("cc", "s")
_ces = _ces.Replace("dd", "t")
_ces = _ces.Replace("ee", "u")
_ces = _ces.Replace("ff", "v")
Return e
End Function
and I'll let the reader work out how badly wrongheaded this one is:
Dim nodelist As XmlNodeList = RXMLSearchTerms.SelectNodes("//foo[@id='" & foo & "']//bar[@id=" & bar & "]//baz[@id=" & baz & "]")
If nodelist.Count >= 1 Then
Dim qry1, qry2, qry3, qry4, qry5 As String
qry1 = ""
qry2 = ""
qry3 = ""
qry4 = ""
qry5 = ""
For Each node As XmlNode In nodelist
Dim nodeReader As XmlNodeReader = New XmlNodeReader(node)
While nodeReader.Read()
If nodeReader.NodeType = XmlNodeType.Element Then
If nodeReader.Name = "querytxt" Then
Select Case nodeReader.GetAttribute("id")
Case "0"
qry1 = nodeReader.ReadString()
Case "1"
qry2 = nodeReader.ReadString()
Case "2"
qry3 = nodeReader.ReadString()
Case "3"
qry4 = nodeReader.ReadString()
Case "4"
qry5 = nodeReader.ReadString()
End Select
End If
End If
End While
nodeReader.Close()
Next
Dim queries() As String = {qry1, qry2, qry3, qry4, qry5}
Dim randomQry As Random = New Random()
qry = queries(randomQry.Next(0, queries.Length - 1))
End If
We had to deploy some special-purpose server written by guys who obviously were reading Stroustrup (and "learning C++") in parallel. E.g., recently-changed files were filled with template stuff, while older files did not use templates. Earliest files were basically C with classes.
I had an annoying bug in file which was written while its author was reading a chapter on exceptions. So, of course, all control-flow was done with exceptions! Plain (sequential!) integers were used in throw statements:
if (fopen(...) == NULL)
throw 2;
if (fread(...) < nread)
throw 3;
/* ... */
if (fclose(...) == EOF)
throw 16;
(Of course, the chapter on iostream is much later in the book!)
The worst codes I've had to maintained were usually C+ codes (i.e. C paradigms wrapped in a C++ Object-in-name-only syntax), where the developer refused to use standard API functions, and where they refused to use language features. Examples:
//java and C#
try {
doSomething();
} catch(Exception e) {}
What you see above is exact--empty catch with no comments!!!!!!
object parser = null;
object result = null;
if(cmd == "cmd1") {
parser = new Cmd1Parser();
((Cmd1Parser)parser).parse(data);
result = ((Cmd1Parser)parser).getStringResult();
} else if( ....) {
...
} else if(cmd == "cmdN") {
parser = new CmdNParser();
((CmdNParser)parser).parse(data);
result = ((CmdNParser)parser).getStringResult();
}
No, CmdNParser's do not inherit from a common "Parser" interface. No, they don't use a common base class. No they don't use composition to re-use the same code. Just copy-paste.
//Note this is languages that support generic programming, C++, Java, C# with no
//legacy code to support.
class Utils {
public static String getString(java.util.List list, int index) {
try{
if(list!=null) {
if(index>=0) {
if(index<list.size()) {
return (String)list.get(index);
} else {
if(log.isInfoLog()) log.info("index invalid.");
return null;
}
} else {
if(log.isInfoLog()) log.info("index invalid.");
return null;
}
} else {
if(log.isInfoLog()) log.info("list null.");
return null;
}
} catch(Exception e) {
if(log.isInfoLog()) log.info("exception.");
return null;
}
}
}
Yes, it was really nested this way. Yes, every log statement was guarded (even though there's no concat). Yes, there's an empty catch block. Yes, null is also a valid element in the list. Yes, all the code is written this way.
And my personal favorite:
class MyClass {
public static ERRNO methodA(StateObject o, Parameters...);
public static ERRNO methodB(StateObject o, Parameters...);
public static ERRNO methodC(StateObject o, Parameters...);
public static ERRNO methodD(StateObject o, Parameters...);
}
WTF?!?!?! This is NOT a utility class...
DELETE
FROM [Order]
FROM [Order]
INNER JOIN Customer ON [Order].CustomerID = [Order].CustomerID
WHERE Customer.Code = @Code
I've lost far to much data to delete statments like this over the years.
I once took over a set of Perl scripts that was someone's first programming project. I don't remember much, but one block that I recall being representative of his programming style was:
if ($day eq "Monday") { print "Monday" }
else {
if ($day eq "Tuesday") { print "Tuesday" }
else {
if ($day eq "Wednesday") { print "Wednesday" }
else {
...
There were dozens places in the code where if statements nested ten or twelve levels deep. He had completed the project, then left for another job... I was stuck with maintaining these scripts. I cursed his name whenever I had to track down a bug in that jungle.
Null checking ad-naseaum.
int CreateObjectX(X** object, int param) {
try {
*object = new X(param);
if(*object!=NULL) {
*X->init();
return 1;
} catch(X_Exception) {
log("error X_Exception.");
if(*object!=NULL)
delete *object;
} catch {
log("error X_Exception."); //honestly, WTF?! Just supress a bad_alloc???
} return 0;
}
object* x1 = NULL;
int res = CreateObjectX(&x1,1);
if(res) {
if(x1!=NULL) {
x1->DoSomething();
if(x1!=NULL) {
delete x1;
}
}
}
//who cares if it's correct... all the null's have been checked!!!!
/*javascript*/
function ltBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lrBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lrbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function ltbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function ltrBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function tBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function trBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function trbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function tbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function rbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function ltrbBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function noneBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function rBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function bBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function ltroBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lroBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function lrboBold(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 3;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function noBoldiVert(iRow, iCol, nRow, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iLeft).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iRight).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iBottom).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iTop).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders(iVert).Weight = 1;
excel.Range(excel.Cells(iRow, iCol), excel.Cells(nRow, nCol)).Borders.Color = "Black";
}
function bBold(iRow, iCol, nCol) {
excel.Range(excel.Cells(iRow, iCol), excel.Cells(iRow, nCol)).Borders(iBottom).Weight = 3;
}
There was a case where 1 was false, and 2 was true. It was used in a 'deleted' column. So in code you would see
if (row.Deleted==1)
// do stuff which one would assume mean't deleted
else
// do stuff which one assumed not deleted
I can't post the code due to its size and NDA-ness...but you'll get the idea pretty quick.
Thousands of lines of code. I could count the comments in TOTAL on my fingers.
I was a poor commenter before that job, and since I have been accused of over-commenting...I wouldn't wish maintaining that code on anyone.
It works, but some of the ugliest:
using perl, parse HTML files that contain table definitions (exported from sqldeveloper) and create VB class definitions, including the SQL to select, insert and update.
I first learned about OOP when I was working on a registration form for a website in PHP. The form was pretty elaborate, several pages total, probably a thousand LOC. I contained all the code in a RegistrationForm class, and I was proud of it.
Then changes came. The client wanted both a registration form and an update form. Both shared so much code that I decided to copy my Registration class, make a few changes, and call it an UpdateForm class. I thought I was being so smart.
Then more changes had to be made to both forms.
More changes.
And more.
Lesson learned: Code duplication is the devil.
I worked on table ( not created by me...) were you have code1, code2..., code7.
For each codex, you have amountx.
Holes weren't permitted.
If you have 2 codes, they must be into code1 and code2.
You can imagine what happens when you delete a set of data. You can imagine also what happens when you need to to do calculations on the amounts...
Instead of writing a lot of crapy code, I should have "fight" against the analyst to create a better data design.
Too late now...
A few weeks ago I found something like:
string number = 1.ToString();
Of course when I saw it again I instantly shocked, checked out, fixed, checked in... but yeah, it sucked.
A class that I wrote had a method named GeneratePolySplitCommandsForSphereTrisectingSharedEdgeOfTwoTrianglesOrOneTriangleAndOneQuadrilateral.
What I thought that was a very smart pattern turned out to be more difficult that I expected to maintain and understand...
It was a list of functions in Python which output could be another function to do further processing or a false value if it couldn't handle the input.
Something like this:
def foo(x):
if testfoo(x):
return lambda y: morefoo(x,y)
def bar(x):
if testbar(x):
return lambda y: morebar(x,y)
def etc(x):
if testetc(x):
return lambda y: moreetc(x,y)
list_of_funcs = [foo, bar, etc]
def some(l):
for i in l:
if i:
return i
f = None
for i in input:
if f:
f(i)
f = None
else:
f = some(g(i) for g in list_of_funcs)
Smart overflow = dumbass.
When I first started programming in Visual Basic, I used to create lots of invisible Label controls just to store data in their Tag property. That is, until I discovered that I could declare properties and fields in the modules themselves...
Once I wrote the following:
char *buf;
assert(buf = new char[length]);
buf[0] = ...
This was crashing with a segfault at that last line. In release mode. Go to debug mode, everything works. Back to release mode, it crashes. The worst kind of error you can have.
Took me a while to realize that the assert (and everything in it) was being removed in release mode. Lesson: never write error-checking code that at the same time has side-effects.
if(file_exists($file4)) { incFile($fName, $file4); }
elseif (($fType2 = $gf->attributes()->type) != null)
{if ($fType2 == 'jquery') {if(file_exists($incFname ="$locFolder/jquery.$scanArea.$fName"))
{incFile('js', $incFname);}}}
All for a simple jquery program. Oh, and the navigation ran off a file-finding snippet in the html. About half a year ago I rewrote it in an OOP style with XML config, not flat file discovery (on runtime), and it's happily running about four websites, including my own.
Oh, and when debugging:
if (1==1){//$logged_in==1){
$user = do_something()
echo $logged_in
return $user
}
PHP file scanning (particularly with version 4), isn't that fun.
It's a tie between
if (char < '0' && char > '9') doStuff();
and
if (char >= '0' || char <= '9') doStuff();
The first was never true and the second was never false. Gee, I wonder why?
In my sordid past, I once wrote a game in which you battled zombies in a cemetery. Zombies had states like this:
#define ZOMBIE_STATE_ALIVE 0
#define ZOMBIE_STATE_SPAWNING 1
And so forth. The switch/case state handling was certainly bad enough, but the worst part is that when a new enemy type was added, the following constant appeared:
#define ZOMBIE_STATE_ZOMBIE_IS_ACTUALLY_A_HAT 8
It is a good thing that we tend to learn more from our failures than from our successes, but I still can't shake the feeling that I will never quite repay my debt to technology. On the plus side, the code/game saw no public release.
Try programming a game before you know what variables are.
I succeeded.
I was using VB.net and basically what I ended up doing was creating little textboxes all over the place, and storing numbers in them. Variables baffled me for at least a few minutes after I discovered them: "but they're invisible!!"
After discovering variables, but not arrays and then being told a handful rules of thumb (global variables are evil, break your code up into functions), I wound up with functions that took about ten or more parameters just so that I could get all the data I needed into them, and I had dozens and dozens of variables named like weapon1name, weapon1dmg ... this is what happens when you get ahead of the class and your teacher doesn't teach you enough :\ Teach me one little concept and I'll take it to the clouds.
I remember my very early days, playing with VB. I wrote this bit that still makes me smile!
On Error Goto Hell
//my code
Hell:
someVar = (1000d / (((value > 1000) ? 1000d : value) < 1d) ? 1d : value)
as a property setter. Looks fine aside from the terseness right? Look at it a while longer...
And i was wondering why it wasn't behaving right!
This isn't exactly the code, but this is the gist of it. This was in a shared library that we were forced to use at a workplace.
public class CustomException : System.Exception
{
public CustomException()
{
}
public CustomException(Exception exception): base(message)
{
try
{
//log something
}
catch
{
throw new CustomException(this);
}
}
}
Just seen this one on stack overflow : http://stackoverflow.com/questions/1611641/unable-to-pause-the-loop-how
As a joke, I once recently wrote this. Please note that there were no comments originally.
import flash.utils.*;
import flash.display.*;
function printIt(event:MouseEvent):void {
var mc:MovieClip
var th:MovieClip =
// This line fires a constructor which then types an object by a parent class
// then it sets the variable mc to that object, cast as its original class
// after adding it to the root.
( mc = ( root as MovieClip ).addChild( new MovieClip() ) as MovieClip ).addChild
(
// new implies that the next three lines or so will translate
// into a Class.
new (
//Convert the string class name to an object
getDefinitionByName(
// Do a string look up of this object's class name
getQualifiedClassName( this )
// It now has to be turned into
//a constructor
) as Class
// Treat the above as a function and cast that as
// a DisplayObject
)() as DisplayObject
// Cast the result into a MovieClip which is stored in the th variable.
)as MovieClip;
//Scales and rotates a MovieClip in the most obscure way possible.
mc.getChildAt( 0 ).transform.matrix = new Matrix( 0, 1.07, -1.07, 0 );
var txtFld:DisplayObject
var txtFld2:DisplayObject
for( var i:int = 0; i < numChildren; i++ )
{
// Notice the two assignments inside of the if statement?
if( !( txtFld = getChildAt( i ) ).hasOwnProperty( "text" ) ||
!( txtFld2 = th.getChildByName( txtFld.name ) ) ||
!txtFld2.hasOwnProperty( "text" ) ) continue;
//Followed by the string look-up of the property?
txtFld2[ "text" ] = txtFld[ "text" ];
}
var printer:printEx = new printEx(mc);
( root as MovieClip ).removeChild( mc );
}
Right now I give my worst in SQL coding, I can't seem to understand that "IN" is a bad thing and that JOINs are not the solution to all problems on Earth. Nor are heavily nested SELECTs. :/
Writing my first-ever game - it was a simple "Scorched Earth"-style tank shooter. As the player adjusted the angle of the tank barrel, I calculated the position of the end of the barrel thusly:
if barrelAngle < 5 then
barrel.x = 20;
barrel.y = 0;
else if barrelAngle < 10 then
barrel.x = 20;
barrel.y = 2;
else if barrelAngle < 15 then
barrel.x = 19;
barrel.y = 3;
.
.
.
else if barrelAngle <= 90 then
barrel.x = 0;
barrel.y = 20;
end if
In my own defense, I was in 9th grade, had been programming for 3 months, and had no idea what trigonometry was. I literally came up with the coordinates using a compass and a ruler on graph paper.
In java, indentation hell:
1.
int x = 10;
2.
if(x < 0)
3.
System.out.print("anywhere");
4.
else if(x < 5)
5.
if(x == 5)
6.
System.out.print("here");
7.
else if (x>=5)
8.
System.out.print("there");
9.
else
10.
System.out.print("somewhere");
11.
else
12.
System.out.print("nowhere");
It would have to be the code I wrote to handle the "array" for my first Tetris game (C++). You see, at the time, I didn't know what an array was. So instead of using an actual two-dimensional array to store the contents of each box on the screen, I created a LOT of variables (170 to be exact):
int square1_1;
int square1_2;
//...
int square17_10;
Since getting and setting the contents of each box was obviously going to become problematic, I developed two functions; one to get and one to set (since I didn't know what a reference or a pointer was, either). The get function, for instance, looked like this:
int getSquare(int x, int y)
{
if (x == 1 && y == 1)
return square1_1;
if (x == 1 && y == 2)
return square1_2;
//...
}
So yeah, it was a hideous mess of copy/paste to say the least; though it ended up working pretty flawlessly and efficiently as a makeshift array. Of course, once I learned about arrays shortly thereafter, all I could do was laugh at this ridiculous, 100kB+ source file that only produced a basic tetris game.
This question needs a reference to the current Not Invented Here comic strip:
Wow, I can't believe nobody has yet mentioned writing OLE (Object Linking & Embedding, later COM) code in C++. Without a doubt the most hideous code I've ever written. A sample:
WordApp.OlePropertyGet("Documents").OleProcedure("Add");
Variant WordDoc = WordApp.OlePropertyGet("ActiveDocument");
Variant MergeDoc = WordDoc.OlePropertyGet("MailMerge");
MergeDoc.OleProcedure("OpenDataSource", TempFileName);
MergeDoc.OleProcedure("EditMainDocument");
WordApp.OlePropertyGet("Selection").OleProcedure("InsertDateTime", "MMMM dd, yyyy", true);
WordApp.OlePropertyGet("Selection").OleProcedure("InsertParagraphAfter");
WordApp.OlePropertyGet("Selection").OleProcedure("MoveDown");
WordApp.OlePropertyGet("Selection").OleProcedure("InsertParagraphAfter");
WordApp.OlePropertyGet("Selection").OleProcedure("MoveDown");
MergeDoc.OlePropertyGet("Fields").OleProcedure("Add", WordApp.OlePropertyGet("Selection").OlePropertyGet("Range"), "FULLNAME");
If you think this is bad, you should see some of the original C code that was required back when OLE was first introduced by Microsoft.
The "treegrid" code in the Visual Studio debugger are my badge of shame. It's a piece of UI that was used in the Watch, Autos, Locals, This, Callstack, and Threads windows. This code shipped first in Visual Studio .NET (2002).
We were working on a new, language-agnostic development environment to replace the previous language-specific environments used by Visual Basic, Visual C++, and Visual J++. At the time there wasn't a workable UI control that could display nested information where both the leaf and internal nodes have the same kind of information. (Compare to the Windows Registry Editor, where the left pane has a tree of "keys" with no properties, and the right pane has a list of "values" with properties, called the "data" and "type". Of course, each key as a "default value" which is kinda the properties of the key, but anyway...)
There were bits of UI that were filling the same niche in VB and C++, but neither was suitable for VS. The VB one was too simple; the C++ one was tightly coupled to the C++ debugger's expression evaluator. So we started from scratch.
I was 6 months out of college, with no experience writing a real, full-sized feature in a green field.
Looking back, I wish I had had a mentor. Specifically a mentor who liked writing UI code. Many programmers avoid UI, so I was out of luck.
I also wish I had understood object oriented programming, specifically the idea that OO programs are composed of simple objects, not functions. It took me years to work that out.
TDD and Refactoring would have helped, but not without a mentor and an understanding of OO and the value of simplicity.
One problem, of course, was feature creep. For example try this:
Notice how the Name and Value columns change size, while the Type column stays the same. Do it again with the Callstack window and see how only the Name column is elastic, while the arrow and language columns are static.
Surely we could have shipped without that, at least for one version, right?
Also, note that some fields are text while others are graphics. We could have skipped that for one release, I think.
For the last decade or so, this code has been the misery for some poor developer. Everyone one of them has (wisely) avoided touching the code as much as possible, making only the most critical changes.
In some sense I did have to answer for my crimes, as I eventually was the manager of the Visual Studio debugger team, so one of my direct reports had to deal with the code I created. I apologized to him every time we spoke about the watch window.
I don't really know why I did this. (C#)
try
{
// Some normal-looking code
}
catch { throw; }
How about this one? (sorry for my java)
Bool isNumber(String string){
if (string.Replace("0","").Replace("1","")./*and so on..*/.Replace("9","").length == 0)
return true;
else return false;
}
I once wrote a Shooting Gallery game in QBASIC. It was in '97. I still have the code lying around. Here is a snippet. I think I am trying to animate the gun, the target, and the bullet at the same time and also performing collision detection. The only thing I remember was that I had three animation loops and I used GOTO with labels to jump in and out of all three of them.
Of course that's not the least of it. There are a million things wrong with this code :). Ahh... sweet nostalgia.
ShootDisk: 'Collision Check
SCREEN 9
LOCATE 25, 2
PRINT "Score:"; Score
LOCATE 24, 26
PRINT "Shooting Gallery(TM) (C) 1997--->"
PUT (GunX, GunY), Gun, PSET
DiskMovt:
k = 0
FOR DiskX = 500 TO 0 STEP -1
PUT (DiskX, 0), Disk, PSET
FOR i = 1 TO 250
NEXT i
IF k = 1 THEN GOTO NextBulletYDisk
GOTO GunMovtDisk
NextDiskX:
NEXT DiskX
GunMovtDisk:
U$ = INKEY$
IF U$ = "4" THEN GOTO GoLeftDisk
IF U$ = "6" THEN GOTO GoRightDisk
IF U$ = "X" OR U$ = "x" THEN CLS : GOTO ScoreDisplay
IF U$ = "F" OR U$ = "f" THEN FOR f = 100 TO 200 STEP 20: SOUND f, 1: NEXT f: GOTO FireDisk
IF DiskX <= 10 THEN CLS : GOTO TargetSelect ELSE GOTO NextDiskX
GoLeftDisk:
GunX = GunX - 7
IF GunX < 5 THEN GunX = GunX + 7
PUT (GunX, GunY), Gun, PSET
GOTO NextDiskX
GoRightDisk:
GunX = GunX + 7
IF GunX > 400 THEN GunX = GunX - 7
PUT (GunX, GunY), Gun, PSET
GOTO NextDiskX
FireDisk:
FOR BulletY = GunY - 10 TO GunY - 219 STEP -1
PUT (GunX + 35, BulletY), Bullet, PSET
IF GunX + 35 >= DiskX AND GunX + 35 <= DiskX + 50 AND BulletY <= 35 THEN PLAY "o0l32cdeddedf": Score = Score + 20: CLS : GOTO TargetSelect: ELSE GOTO ContinueDisk
ContinueDisk:
k = 1
GOTO NextDiskX
k = 0
NextBulletYDisk:
NEXT BulletY
GOTO GunMovtDisk
This one is something I came across recently. I have no idea why I wrote it:
if(defaultChargeUsed) {
shippingCharges += charge;
responsePackage.setShippingCharge(charge);
}
else {
shippingCharges += charge;
responsePackage.setShippingCharge(charge);
}
I once noticed my data was generating duplicates, so I added a loop to check each chunk of data to make sure I wasn't adding it twice:
foreach (string x in currentdata)
{
if (!existingdata.ContainsKey(x))
{
existingdata[x] = true;
DataHelper.AddData(currentdata);
}
}
The database needed several hours of scrubbing to clear out the mass of duplicate junk this mistake caused. The original code had called DataHelper.AddData(currentdata); and I failed to change that to DataHelper.AddData(x); when I added the loop.
In highschool, buying a Ti-83 calculator was mandatory for math class and that's how I started programming. I tried making a chat program by asking the user for input, writing that input to an image and sending that over a wire to the other calculator. All in glorious TI-BASIC. Now, the WTF is that I had the same couple lines of code for every character that the user could input. I think the code for the input ended up occupying like 30KB, of the available 64KB. I remember it being quite a glorious revelation when I realized that that's what loops are for.
1999ish, as part of a much larger app, someone asked me to create an interface where you could put in very simple logical statements, then save the generated statement back to a database. I built something very simple to fill the need. They added a requirement, I patched it on. At some point, I knew this needed a rewrite, and got told I was an idiot for even suggesting that.
So it kept going. And going. And going.
To a point where bugs couldn't be fixed, and we'd built a parser for a complex language... without ever really thinking about it as a parser, which meant, among other things, that it was pretty embarrassingly busted.
Lesson: if the boss calls you an idiot and seems to mean that, don't wait to start looking for another job. You're going to compromise yourself professionally and personally if you stay.
Several years ago I was working as a Object Pascal (Delphi) Programmer.
Back then, I tried to avoid using OOP (don't ask me why). I implemented an extreme WTF: all fields from database was encapsulated into delimited strings (mind that I didn't want to use objects). It was insurance costing application, so users needed to calculate cost sheets. I ended up with straight-out-of-hell function which summed all the costs: an extreme maze of if-else statements. Of course it was hard to understand, yet to debug. One condition failed at runtime for no particular reason (I hunt it down for a few days) showing a message "That should never happen" to one of stakeholders. It was the final straw, I decided on rewriting this part using Object Oriented approach.
What I learned from this is:
Well I've found this piece of code in a bank administration source code, I called if forswitch
for (int i=1; i<=10; ++i) {
switch(i) {
case 1:
doSomething();
break;
case 2:
doSomething();
break;
case 3:
doSomething();
break;
case 4:
doSomething();
break;
case 5:
doSomething();
break;
case 6:
doSomething();
break;
case 7:
doSomething();
break;
case 8:
doSomething();
break;
case 9:
doSomething();
break;
case 10:
doSomething();
break;
}
}
My personal best I created when I was working simultaneously with MATLAB and C++ and it was:
int sizeOfArray = 100;
int[] myArray = new int(sizeOfArray);
...
...
...
memset(myArray, 0, sizeOfArray * sizeof(int));
It was placed in a quite large class and it took me quite a while to figure this out.
I once wrote a function accepting a pointer as an argument:
void getElements(int *elems , int numElems)
{
int *tempElems = calloc(numElems,sizeof(int));
elems = tempElems;
//getting elements into tempElems
}
For a long time, I couldn't figure out why I wasn't getting my elems filled in the main() function. That was until I realised I was merely passing the pointer elems, instead of its address.
A mess of PHP, no attempt to separate logic from views, the same SELECT statement and row print loops copy and pasted into a "search function" and the different views that needed to be updated individually for each change, and a scary implementation of sessions.
I heard that the next developer took one look, shook his head and rewrote it intelligently.
int j = 0;
.....
// some time later:
for (int i=0; i<max; i++)
{
j = j + 1;
}
I'd refactored the hell out of the code, and everything was eventually taken out of the loop. Of course, I left one bit opf factoring out...
I always refer to this as a bit of code 'I once saw'. Nobody needs to know that it was me that created it!
once i saw a code like this:
IF(true)
{ }
ELSE
{
tons of code!!!
}
Code that was deleted or never executed due to requirement changes...
Simple:
private bool ValidateUser( string userId , string passwordGuess)
{
User u = User.Load(userId);
if (u.Password != passwordGuess) // Check passwords match
return true;
else
return false;
}
Lesson, always test your code :-)
It's funny having users complain their passwords don't work, which shows they do know what they're typing :)
In C# I have a logging subsystem that filters log messages by the method and class. The logging subsystem uses the StackFrame class to obtain the invoking method. StackFrame is expensive to use (about 1ms on my dev machine), and is way too slow to be constructing on every log message.
The solution was to create a cache of StackFrames, keyed off of some sort of magical token that is unique for every invocation of the Log functions. Here's what I came up with (simplified for brevity):
class Log
{
public delegate void VoidDel();
Dictionary<int, StackFrame> stackFrameCache = new Dictionary<int, StackFrame>()
public void Log.Message(VoidDel token, string message, params object[] vals)
{
int invocationId = token.Method.MetadataToken;
StackFrame frame;
if(!stackFrameCache.TryGetValue(invocationId, out frame))
{
// Get the stack frame for the invoking method and put it in the cache
}
// use the stack frame to filter and format the log message
}
}
Then, the really horrible part is that every Log message invocation requires a magic incantation:
Log.Message( ()=>{}, "Something Happened" );
The lambda function ()=>{} is guaranteed to be unique to that line of code, with a unique MetadataToken value.
It works, it's fast (approximately 300x faster than generating a StackFrame on every call), and if anyone ever did it in production code I'd fire them.
This is not exactly answering the question, but many years ago I maintained an online IBM Mainframe CICS system written in COBOL. There was one program, a parser, that was used every time a transaction came in, and it was written by a former Assembly programmer who had never heard about subroutines, and apparently was trying to reduce his risk of carpal tunnel syndrome by making most of his variables and arrays have one-letter names. Oh, and GOTO, he loved GOTO. It was the worst example of spaghetti code I have ever seen. The programmer I replaced told me that if I was ever asked to make a modification to that program to fight with my life to avoid it.
Not taking his advice, one day I needed to make a modification, and I made a very innocuous one-line change in the program, only to find that every time it ran it didn't simply crash, but it took the entire CICS region down with it. Once I determined that the reason the region had come down was because of the simple change I made, I saved off the code, and then made my needed change in a different way. That worked, but because my initial foray could take a region down, I kept its code because to make certain changes take effect in that verison of CICS, the region needed to be rebooted, and occasionally the sysadmin was not available. So, I had a reboot utility! I never did figure out how it could do what it did.
This happened the other day. It was about three in the morning, I was tired, what can I say. I wrote this in C/C++:
int i=0;
while(i<0)
{
//some code
i++;
}
I wondered why it wasn't working, it took me about a minuter to figure it out. Then I remembered this question and I had to post it here.