What Could Go Wrong



  • So in this application I inherited the database can only be described as a steaming pile of ****. Today I found yet another gem from the previous programmer.

    The users table has no primary key column but it did have an identity surrugate id column. Which I promptly set to PK with a single mouse click (so much work right!). All 16 other columns in the table are nullable with no constraints.

    The real fun begins though when I saw the code for the webform that creates/updates users. Process goes something like this:

    1. Admin fills out new user details and clicks a button to call an ajax function to create the user record in the background.
    2. If no user id is passed in (ie a new user) the system create a new user record by doing an insert "with all data cols set to null" and returning the new userId.
    3. Once the new userId is found an update is run to set the other 16 column values.
    4. Finally the userId is returned to the caller and the system closes the web form

    In addition transactioning is not used EVER.

    Can anybody else see how a web based app, using ajax, no transactioning and no constraints could create problems? Like say 370 duplicated users some with as many as 6 duplicates.

     



  • I'm working on a project where I'm having to implement everything from the ground up from scratch. This is the paradigm I think I'll go with; you're right. What could go wrong? It's perfect! </sarcasm>



  • Jesus...  Until you mentioned ajax, I was about to go and ask my boss if he added you as another developer to our app rewrite.  :)

    The database I inherited (and am promptly killing with fire) sounds just like what you describe, including the lack of any constraints, lookup tables only containing 'Y' and 'N', little or no relationships, empty strings and null scattered in free-form fields which should be ID fields, etc., etc.  The first day I was on the job, I was able to add myself to the system three times (I stopped after the third attempt), and performed (very easily) sqli and xss attacks...  ON THE FIRST FUCKING DAY!

    Man, am I ever glad I'm rewriting this heaping pile of shit.  Oh, and just to give you a taste of the code-base, check out this beaute:

    Session["SomeKey"] = (DataTable)null;

    Oh yeah, they're setting a data table in session to a null of type DataTable...  Hmm, didn't know you can have nulls of a specific type.



  • @C-Octothorpe said:

    Hmm, didn't know you can have nulls of a specific type.



    It really doesn't make sense in this case (or most cases), but there are some cases in which you need to cast a null. For example:

    
    public void SomeMethod(ReferenceType param)
    {
    ...
    }
    
    public void SomeMethod(AnotherReferenceType param)
    {
    ...
    }
    
    

    If you call SomeMethod(null), how does the compiler decide which method to call? If you call SomeMethod((ReferenceType)null), then it knows to call the first one.
    It's not exactly a "null of a specific type", it's really just a compiler hint in that case.
    But yes, your example is completely stupid.


  •  With the exception of the web form (a delphi program is used instead) this sounds exactly like a product I worked on during my first internship.  Whats worse, the product was an EMR (electronic medical record) system...for pediatricians...and it was supposedly one of the better ones out there.  I cant even begin to count the number of calls I got from nurses saying there were multiple records for the same patient in the system.  A couple times their patients came dangerously close to not getting their prescriptions filled because of this.



  • @C-Octothorpe said:

    Jesus...  Until you mentioned ajax, I was about to go and ask my boss if he added you as another developer to our app rewrite.  :)

    The database I inherited (and am promptly killing with fire) sounds just like what you describe, including the lack of any constraints, lookup tables only containing 'Y' and 'N', little or no relationships, empty strings and null scattered in free-form fields which should be ID fields, etc., etc.  The first day I was on the job, I was able to add myself to the system three times (I stopped after the third attempt), and performed (very easily) sqli and xss attacks...  ON THE FIRST FUCKING DAY!

    Man, am I ever glad I'm rewriting this heaping pile of shit.  Oh, and just to give you a taste of the code-base, check out this beaute:

    Session["SomeKey"] = (DataTable)null;

    Oh yeah, they're setting a data table in session to a null of type DataTable...  Hmm, didn't know you can have nulls of a specific type.

     

    Actually I insisted that management hire another dev to help me fix this system because of the size of the issues and that I be involved in the interview process. We had a great guy lined up, really knew his stuff. He took one look at the code and quit.

     I kid you not. He said it was for other reasons but I know it was the code.



  • Really, I don't blame him though...  Unless the rate or location is crazy good, I learned a long time ago to try to have a look at their code base before agreeing to anything.  Just a sampling of some of their 'best' code usually is enough.

    So are you rewriting or maintaining this POS?



  • @toth said:

    @C-Octothorpe said:
    Hmm, didn't know you can have nulls of a specific type.

    It really doesn't make sense in this case (or most cases), but there are some cases in which you need to cast a null. For example:
    
    public void SomeMethod(ReferenceType param)
    {
    ...
    }
    

    public void SomeMethod(AnotherReferenceType param)
    {
    ...
    }


    If you call SomeMethod(null), how does the compiler decide which method to call? If you call SomeMethod((ReferenceType)null), then it knows to call the first one.
    It's not exactly a "null of a specific type", it's really just a compiler hint in that case.
    But yes, your example is completely stupid.

    I see what you're saying, but in this case I would create a parameterless overload.  But if it's an API you have no control over, yes, you're absolutely right and you may be passing in a "casted" null.  The thing I was trying to highlight in my code sample was the complete lack of understanding, not the edge case that you mentioned a la Hanlon's razor...



  •  @C-Octothorpe said:

    Really, I don't blame him though...  Unless the rate or location is crazy good, I learned a long time ago to try to have a look at their code base before agreeing to anything.  Just a sampling of some of their 'best' code usually is enough.

    So are you rewriting or maintaining this POS?

    Unfortunately I'm maintaining it since "We do not have sufficient resources to consider a rewrite". What I'm basically doing now is that whenever a bug report comes through I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.

     



  • @codefanatic said:

     @C-Octothorpe said:

    Really, I don't blame him though...  Unless the rate or location is crazy good, I learned a long time ago to try to have a look at their code base before agreeing to anything.  Just a sampling of some of their 'best' code usually is enough.

    So are you rewriting or maintaining this POS?

    Unfortunately I'm maintaining it since "We do not have sufficient resources to consider a rewrite". What I'm basically doing now is that whenever a bug report comes through I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.

    Well, do whatever you can, on their time of course, to make your day-to-day life easier.  Unfortunately, it sounds as if there are some seriously big (read architecture) problems which I suppose will have to wait until they have more "resources"...

    Good luck and godspeed.



  • @codefanatic said:

    Its painful but I am slowly winning the battle...or going insane. Not sure which.

     

    From the sound of it, either one would be better than letting this code survive.

     



  • @lethalronin27 said:

    @codefanatic said:

    Its painful but I am slowly winning the battle...or going insane. Not sure which.

     

    From the sound of it, either one would be better than letting this code survive.

     

    He is for sure a much better person than I am. At my new job, I decided to work on a new code base in parallel to the existing one; I just didn't want the hassle of trying to untangle this mess -- if only because understanding how far reaching some things are and trying to ensure to not introduce WTF bugs at the same time... it just seemed more sane to start from scratch and only touch the old stuff when absolutely necessary.


  • Considered Harmful

    @toth said:

    If you call SomeMethod((ReferenceType)null), then it knows to call the first one.

    Or SomeMethod( default( ReferenceType ) )



  • @dohpaz42 said:

    @lethalronin27 said:

    @codefanatic said:

    Its painful but I am slowly winning the battle...or going insane. Not sure which.

     

    From the sound of it, either one would be better than letting this code survive.

     

    He is for sure a much better person than I am. At my new job, I decided to work on a new code base in parallel to the existing one; I just didn't want the hassle of trying to untangle this mess -- if only because understanding how far reaching some things are and trying to ensure to not introduce WTF bugs at the same time... it just seemed more sane to start from scratch and only touch the old stuff when absolutely necessary.



     I tried to do that at the start only to find out that the previous programmer was
    still working on the code (despite me being told otherwise when I
    started). This made keeping the changes in sync impossible. Now I try to limit his influence as I fix things.

     



  • @C-Octothorpe said:

    in this case I would create a parameterless overload

    Presumably, the parameterless overload would still need to call one of the methods with a null value. Unless you implement it by copy-pasting the method body each time, in which case TRWTF would be you.



  • @codefanatic said:

    Unfortunately I'm maintaining it since "We do not have sufficient resources to consider a rewrite". What I'm basically doing now is that whenever a bug report comes through I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.
     

    What you need is a hard drive "failure" to wipe out the old codebase. Then you'll have no choice but to rewrite it correctly.

     



  • Dude!  Front page material!



  • @codefanatic said:

    I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.
     

    That's an inclusive or.



  •  @da Doctah said:

    @codefanatic said:

    I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.
     

    That's an inclusive or.

    You could be right ;-)

    If I didn't have tdwtf to rant on I would definately go insane.

     



  • @codefanatic said:

     @da Doctah said:

    @codefanatic said:

    I factor in enough time to not only patch the bug, but also rewrite the component that contains the bug.Its painful but I am slowly winning the battle...or going insane. Not sure which.
     

    That's an inclusive or.

    You could be right ;-)

    If I didn't have tdwtf to rant on I would definately go insane.

    You're obviously starting to crack...



  • @toth said:

    @C-Octothorpe said:
    in this case I would create a parameterless overload
    Presumably, the parameterless overload would still need to call one of the methods with a null value. Unless you implement it by copy-pasting the method body each time, in which case TRWTF would be you.
    The only time I've ever seen overloads like the ones you have described are when there are multiple overloads, and each is usually pointing to the "main" overload with several parameters which contains all the logic, unless you have a "special" code base you created, in which case TRWTF is you.  See below:

    <FONT size=2 face=Consolas><FONT size=2 face=Consolas></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>public</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>void</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> str, </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>object</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> obj)</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    {

    </FONT></FONT><FONT color=#008000 size=2 face=Consolas><FONT color=#008000 size=2 face=Consolas><FONT color=#008000 size=2 face=Consolas>// real logic is here

    </FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    }

    </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>public</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>void</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> str)</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    {

    Method(str, </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>null</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>);</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    }

    </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>public</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>void</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>object</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> obj)</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    {

    Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>null</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>, obj);</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    }

    </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>public</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>void</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> Method()</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    {

    Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>null</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>, </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>null</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>);</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    }

    </FONT></FONT>

    See the .Net framework core library for more examples...



  • @Mo6eB said:

    @codefanatic said:

    If I didn't have tdwtf to rant on I would definately go insane.

    You're obviously starting to crack...

     

    "If you put an A in definitely, then you're definitely an A-hole."

    http://theoatmeal.com/comics/misspelling



  • Our last ERP system (~30 years in the making, which we used from 2006 to 2007) used the "delete and reinsert without transactions" method of updating data, which I discovered while investigating why our major customers kept disappearing from the database. And when querying any table in the system, you had to filter by a "record type" field or else you'd get corrupt garbage because they had a habit of pointing several tables to the same data file (Pervasive PSQL allows this), so when you queried one, you got records from them all, but with the fields from the other tables being misaligned. Their own code (in ACUCOBOL-GT, which their sales reps conveniently didn't mention) combined all the fields back into a single text field so it was all the same to them, and merging the tables let them query parent and child rows together in one query without any joins.



  • @codefanatic said:

    2.  If no user id is passed in (ie a new user) the system create a new user record by doing an insert "with all data cols set to null" and returning the new userId.

    3.  Once the new userId is found an update is run to set the other 16 column values.

     

    I do something like this in a couple of places, but it's all within a single stored procedure (and my step 4 is "run the resulting row through a standard validation API") so I don't think it's a WTF.  What it is is replacing this:

     

    insert into the_table ( <long list of columns> ) values ( <long list of parameters> )

     

    with this:


    insert into the_table ( <column 1> ) values ( <parameter 1> )

    update the_table set <column 2> =<parameter 2>, ... <column N> = <parameter N> where <it's the row that was just inserted>

     

     



  • @emurphy said:

    @codefanatic said:

    2.  If no user id is passed in (ie a new user) the system create a new user record by doing an insert "with all data cols set to null" and returning the new userId.

    3.  Once the new userId is found an update is run to set the other 16 column values.

     

    I do something like this in a couple of places, but it's all within a single stored procedure (and my step 4 is "run the resulting row through a standard validation API") so I don't think it's a WTF.  What it is is replacing this:

     

    insert into the_table ( <long list of columns> ) values ( <long list of parameters> )

     

    with this:


    insert into the_table ( <column 1> ) values ( <parameter 1> )

    update the_table set <column 2> =<parameter 2>, ... <column N> = <parameter N> where <it's the row that was just inserted>

     

     

    It would'nt be a wtf IF the sql used transactioning and the user editor had some kind of locking of records.

    Hell I'd even settle for a unique constraint and not null setting on the username column.

     


  • Notification Spam Recipient

    found this thread using the recently re-enabled search. It's not 10 years ago, so you know it's legit!

    Also, only three forum members in this thread have an avatar (at time of writing).

    Greets to @error @da_Doctah @mott555 !



  • @C_Octothorpe said in What Could Go Wrong:

    <FONT size=2 face=Consolas><FONT size=2 face=Consolas></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>public</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>void</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> Method(</FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> str, </FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>object</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> obj)</FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    FONT

    size equals two

    face equals

    C

    onsolas

    U+002F SOLIDUS

    FONT

    🐄💨 📖

    This is how you poetry.



  • @ben_lubar

    This is how you poetry:

    Oh freddled gruntbuggly,
    Thy micturations are to me,
    As plurdled gabbleblotchits,
    On a lurgid bee,
    That mordiously hath blurted out,
    Its earted jurtles,
    Into a rancid festering confectious organ squealer. [drowned out by moaning and screaming]
    Now the jurpling slayjid agrocrustles,
    Are slurping hagrilly up the axlegrurts,
    And living glupules frart and slipulate,
    Like jowling meated liverslime,
    Groop, I implore thee, my foonting turling dromes,
    And hooptiously drangle me,
    With crinkly bindlewurdles,
    Or else I shall rend thee in the gobberwarts with my blurglecruncheon,
    See if I don't!



  • @Arantor Nah, that merely inspires revulsion and projectile vomiting. Try this on for size

    i = i++ + ++i;



  • @tufty no, that inspires revulsion and vomiting - and I'm a PHP programmer.

    Then again, the Wordpress people insist 'code is poetry', I can only buttume they mean this kind.



  • @tufty said in What Could Go Wrong:

    @Arantor Nah, that merely inspires revulsion and projectile vomiting. Try this on for size

    i = i++ + ++i;

    That induces E_CHECKIN_PRIVILEGE_REVOKED. Or the more severe E_YOU_ARE_FIRED. (rolls dice)


  • 🚽 Regular

    @ben_lubar said in What Could Go Wrong:

    This is how you poetry.

    Community Server supported manual syntax highlighting.


Log in to reply