Hell is other people's code
by
, April 17, 2015 at 11:56 AM (14156 Views)
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
- 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:
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.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 & Ladders disabled in AdminCP -->\r\n"); print_no_permission(); }- 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.
- 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.