View RSS Feed

The Kraken of TWC

Hell is other people's code

Rate this Entry
I have done a lot of work on this site, including both creating custom hacks and installing other peoples custom hacks and tweaking them to suit our needs. I've released some of the custom hacks I've done so that other vbulletin sites can use them. My biggest gripe is other people writing bad code, sometimes just plain terrible almost unusable code and I end up having to fix it to make it work.

I'll be the first to admit that there's no real concrete set of standards that all coders must follow, especially for plugins as those tend to be written by people like me doing it for fun. But many of the items on the following list you'd think any coder would want to follow.

List of Programming Gripes

  1. Not testing code - you'd think this would be a given you want to make sure that your code actually does what you say it does, there have been a number of times where I've looked at a product and the code just doesn't work and it's not like its on rare cases you wouldn't expect someone never to run into. For example the tournaments add-on has this gem of coding, which caused user profiles to be inaccessible for a while:
    Code:
    if (!in_array(THIS_SCRIPT, array('ladders', 'competitors', 'tournaments', 'teams')) && !$vbulletin){
    	global $vbulletin;
    	print_r("<!-- Non-standard page, trying global vbulletin variable -->\r\n");
    }
    if (!$vbulletin->options['tmnt_active'] && $vbulletin->userinfo['usergroupid'] != 6)
    {
    	print_r("<!-- vBulletin Tournaments &amp; Ladders disabled in AdminCP -->\r\n");
    	print_no_permission();
    }
    Basically what the code is supposed to do is if you are on one of the custom pages for the tournament mod and the mod isn't active only admins should be able to see the page. The problem comes that this bit of code is executed when viewing user profiles. What it actually does is if the tournament mod is not active and you aren't an admin you can't view user profiles.
  2. Not cleaning up after themselves. This isn't limited to vbulletin plugins though I've seen many that do this. Basically if you put something on the system, or in vbulletin's case the DB, when you get uninstalled make sure you remove every single thing you put on. This is why OS's slow down over time because software, including the OS, writes stuff to disk or the registry or other important areas, and it doesn't get removed and this accumulated gunk slows down a machine over time. One of the worst culprits for this is Visual Studio, when you install it rather than having one item in your add/remove programs list it adds dozens, one for each component, but when you uninstall visual studio it only removes a handful of the pieces of software it installs. This is all the more amusing and ironic because to get a .NET application certified by Microsoft the app needs to totally remove itself from the machine, yet the IDE used to create the application can't even do it. For vbulletin this creates issues because the installation code usually can't handle being re-installed if the previous attempt failed and didn't uninstall correctly because of poor coding, which leads to the next item.
  3. Bad Code. I totally get that time is money, but every study done has said that bad planning leads to bad code, which leads to bad testing costs a tonne more money that had more time been spent in the planning phase of writing an app/plugin. Bad code means maintenance of the code and adding new features to the code takes longer is more error prone, harder to test and is less likely to be repeated. The ModSystem plugin for example has a bunch of different settings for each mod, but in the DB they are each stored as a separate integer column. A stupid design decision, the settings should be one integer column that is used a bit field (i.e. each binary digit in the integer column is either on or off depending on if the setting is active). This would mean that one integer could be used to 32 settings instead of needing 32 integers to store 32 settings. Much easier to expand the number of settings required and far less use of space in the DB.


I could go on but I think my rant has gone on long enough now.
Tags: vbulletin
Categories
Life on TWC , Technology

Comments

  1. GrnEyedDvl's Avatar
    I completely agree with Squid on this, though I generally tend to yell about vanilla vBulletin code since that is the piece of software we actually pay for. Someplace around here is a thread with some crappy error trapping in it that just throws "die 4 true/false" in the php logs which they claim was just for developers to know when a certain script died. But they left it in the production version. They know exactly why a script dies, and then log it this way so nobody can figure out what is going on.
  2. Ybbon's Avatar
    And yet the simplest thing to do is one that coders forget to do. Comment, then comment some more, certainly while developing it so you know it hits the line you want, when you want it to. You can always remove the excessive lines afterwards but leave a basic outline of what the code is doing for anyone else reading it afterwards.
  3. Gigantus's Avatar
    I wouldn't even find my way through my M2TW scripts if I didn't comment the stuff - and that is probably the easiest coding out there.
  4. Augustus Lucifer's Avatar
    To tack onto that Ybbon, definitions aren't used often enough in code in general. A simple define("ADMINS", 6); at the top would do wonders to clarify that snippet at the bottom, since 6 is meaningless in a vacuum.

    The three gripes might as well be chiseled into a stone tablet and passed down from programmer to programmer, because they're only getting more common the less adept you have to be to get your feet wet coding. TAOCP and Code Complete are as relevant today as when they were written, but if people focus on just learning the language and not learning best practices then all of these things are what result.

    (There's also the fact that PHP is evil, but it can hardly be blamed for operator stupidity)
  5. Squid's Avatar
    Quote Originally Posted by Augustus Lucifer
    To tack onto that Ybbon, definitions aren't used often enough in code in general. A simple define("ADMINS", 6); at the top would do wonders to clarify that snippet at the bottom, since 6 is meaningless in a vacuum.

    The three gripes might as well be chiseled into a stone tablet and passed down from programmer to programmer, because they're only getting more common the less adept you have to be to get your feet wet coding. TAOCP and Code Complete are as relevant today as when they were written, but if people focus on just learning the language and not learning best practices then all of these things are what result.
    That would probably be #4, don't use magic numbers.

    (There's also the fact that PHP is evil, but it can hardly be blamed for operator stupidity)
    The problem is BKC (between the keyboard and the chair).
    Updated April 28, 2015 at 12:46 PM by Squid
  6. GrnEyedDvl's Avatar
    Quote Originally Posted by Squid
    The problem is BKC (between the keyboard and the chair).