It worked so we just left it



  • I was asked to do a code review of a system that was owned, written and supported by our new overlords.

    While spelunking around their source tree, I discovered a directory with about 37,000 different classes, all identical except for the name. They all had the same prefix and were sequentially numbered with a six digit number. Upon checking source control, I noticed that each and every one of them was updated and committed at the same time every weekend at about Sat 9:30PM.

    ?

    The senior dev explained that when the project began, they tasked one of their offshore developers with writing a script that would query the database for the columns (and types) in a specific table and generate a class to represent it. Nothing fancy. The class would have a default constructor, and one that would take all the columns as arguments, as well as a private member with matching getter and setter methods for each column in the table. Of course, there was some other code that would manipulate a list of these objects. This way, if the table had to change, they could just run the script and rebuild the code.

    After two weeks, the offshore guy proudly came back with a script. When they ran it, it generated tens of thousands of classes, each with the prefix name and a sequential number as part of the class name. It also generated an interface with all the methods in "the" class.

    It turns out, the guy had written it such that it would generate a unique class for every row in the table.

    The loader class would query all the rows required, then compute the name of the desired object (Xyz + formatted row-number), and instantiate and populate the object before adding it to a List<IXyz>. Mind you, the sequence in which the rows were returned from the query was irrelevant; row id 1 could be returned first this time and 10th next time. The specific class to be instantiated was determined by result-set position, not the data.

    The calling code would then spin through the list and manipulate the different classes by the interface. 

    Unfortunately, it all worked. But at the time, they were under the gun and didn't have time to rewrite it, so they just left it.

    I can't wait to see what happens to their six digit number-formatter when they add the 1 million-th row in their table

     



  • Just out of interest, how close are you to crossing the line into "Fuck the money, I have to get out of here for the sake of my sanity" territory...?



  • @snoofle said:

    I can't wait to see what happens to their six digit number-formatter when they add the 1 million-th row in their table

    I bet you can't.  From previous evidence I would imagine it is quite likely that they'll pay you quite a lot to fix it...



  • @Hmmmm said:

    Just out of interest, how close are you to crossing the line into "Fuck the money, I have to get out of here for the sake of my sanity" territory...?
     

    Lately, I'm geting there...



  •  Home-grown ORM! Fantastic.



  • @dhromed said:

     Home-grown ORM! Fantastic.


    ...where the "M" stands for
    (function(marray) {
    var randomIndex = Math.floor(Math.random() * marray.length);
    return marray[randomIndex];
    })(["Mutation", "Massacre", "Misdirection", "Malevolence", "Monstrosity", "Molestation", "Malediction"]);



  • @snoofle said:

    It turns out, the guy had written it such that it would generate a unique class for every row in the table.

    AAAAAAAUGHHHHH

    AAAAAHH AAAAAAAAAHHHHHHH

    NONONONONONONONONO

    MAKE IT STOP



  • @snoofle said:

    It turns out, the guy had written it such that it would generate a unique class for every row in the table.

    ...

    Unfortunately, it all worked. But at the time, they were under the gun and didn't have time to rewrite it, so they just left it.

    Wouldn't the fix for this be to add 'TOP 1' to the select in the script?  Or am I miss understanding and it wasn't meant to be a class to represent a row in said table?



  • @locallunatic said:

    Wouldn't the fix for this be to add 'TOP 1' to the select in the script?
    That's one way.

    In this case, he shouldn't have been querying the table for data; he should have been querying the user-table to get the columns that describe the table (e.g.: column_name, type, nullable) and using that to control the generation of the class.


  • BINNED

    Or not writing his own ORM in the first place?



  • Snoofle if you lived in my area I would buy you a beer.



  • @Anketam said:

    Snoofle if you lived in my area I would buy you a beer.


    I myself would offer him something stronger (and if you're talking about US-American beer that wouldn't even be hard to do :p )



  • The worst part is they could have used Hibernate to generate the classes. No scripting ever needed to be done, and I know your company has used Hibernate before too.



  • Hey, at least a) all of the classes are the same, and b) they all implement the same interface.

    I came across a guy once who thought that every class, without exception, needed to be a singleton.  (Not that he would have known what 'singleton' meant).  He would just copy a class and increment a numeric suffix by hand if he needed more of the same thing.  Of course, over time, he'd change how the class(es) worked and forget to propagate the change to every copy, so maybe Widget32.Foo() and Widget12.Foo() do slightly different things.

    His rationale for not having more than one instance of any class?  "You can't know which one you're dealing with and it's hard to read".

    The following, of course, is easy to read:

     

    DoAllFoos()
    {
      widget1.Foo();
      widget2.Foo();
      .
      .
      .
      widget83.Foo();
    } 


  • Amazing. Simply amazing. It's like an Inverted Paula Bean: instead of doing nothing, doing way too much.


  • BINNED

    @Rhywden said:

    @Anketam said:

    Snoofle if you lived in my area I would buy you a beer.


    I myself would offer him something stronger (and if you're talking about US-American mass-marketed beer that wouldn't even be hard to do :p )
    FTFY (American microbrews are readily available at 6% alcohol and up.)



  • @TGV said:

    Amazing. Simply amazing. It's like an Inverted Paula Bean: instead of doing nothing, doing way too much.

    What a tnallirb idea!

     



  • @Mason Wheeler said:

    What a tnallirb idea!

    Gnodab!



  • @Rhywden said:

    @Anketam said:

    Snoofle if you lived in my area I would buy you a beer.


    I myself would offer him something stronger (and if you're talking about US-American beer that wouldn't even be hard to do :p )

    I think that Dogfish Head might disagree with you on that last point. For those unwilling to click on a link or type your age into a website, that's their 120 minute IPA - each bottle is about 15-20% ABV (the one I got on tap was 18%). There are plenty of beers that have a higher alcohol content, but they're usually not easy to find or get a hold of.



    Big, popular, American beer makers (they are not worthy of being called breweries or anything of the sort) are horrible, but there are some great craft brewers around - Dogfish Head being one of my favorites. There's a brewpub nearby called The Cheeky Monk (in Denver), which has around 30 types of beer on tap - most of them from Belgium. They usually have about three types of Dogfish Head beer on tap, too.



    On the subject: It seems to me that no single drink could handle the level of issues snoofle has been facing. I'm thinking he's in need of some serious absynthe and Black Blood of the Earth shots. Probably at least a dozen.



  • @valczir said:

    I'm thinking he's in need of some serious absynthe and Black Blood of the Earth shots. Probably at least a dozen.

    You will break.



  • @blakeyrat said:

    @Mason Wheeler said:
    What a tnallirb idea!

    Gnodab!

     

    You loathsome creature.


Log in to reply