For those of you who haven't read my initial post on this (see the resource links, because the inline link editor is playing silly buggers again), I suggest that you do. If nothing else, it will probably provide some context. At the end of that, I mentioned I'd probably find more signs as I spent more time working on the system, with the current developers. Unfortunately, I was not wrong about that. Far from it ...
I'm going to start referring to the current system as "Web-based DERP", seeing as that's how I initially misread the name thereof (and it seems appropriate). Bad/inconsistent kerning in the chosen font is the least of my worries. For many reasons, it's very definitely not "simple and brilliant Web-based business software", despite the marketing.
Here, then, are some further discoveries, which I seem to make almost daily, about the depth/extent of the derpiness:
V: Voilà! In view, a humble vaudevillian veteran, cast vicariously as both victim and villain by the vicissitudes of Fate. This visage, no mere veneer of vanity, is a vestige of the vox populi, now vacant, vanished. However, this valorous visitation of a by-gone vexation, stands vivified and has vowed to vanquish these venal and virulent vermin vanguarding vice and vouchsafing the violently vicious and voracious violation of volition.
[carves "V" into poster on wall]
V: The only verdict is vengeance; a vendetta, held as a votive, not in vain, for the value and veracity of such shall one day vindicate the vigilant and the virtuous.
[giggles]
V: Verily, this vichyssoise of verbiage veers most verbose, so let me simply add that it's my very good honor to meet you and you may call me V.
— V for Vendetta; Alan Moore
1. The interaction/relationship between users, user fields, user groups, roles and permissions is overly complex, being both difficult to set up and alter. This is something Drupal got right early on and ran with, so much so that I've copied their model in systems I've created: The users
table should have little more than a username, password, salt for hashing and an is_active
flag. All additional/other attributes should be in an additional user_fields
table, to which one can JOIN
as/when needed. Users should have roles: user_roles
is thus a simple bridging table between user IDs and role IDs. Roles should have permissions: yet another ID-based bridging table. (This makes core login/out and permission CRUD operations fairy quick and simple to do.)
2. DB columns have names like xxxt_groupIS_users_groupsID
(instead of something simple like ugid
), never mind that the case matters and names use a mixture of camelCase, UPPER_CASE and snake_case. Also, despite the use of words like "is" and "has", the values often aren't Boolean in nature (which is misleading).
3. xxxt_Smurf_entity_namingIS_everywhere
: Except for globals (not in a namespace, of course), all class, method, field, attribute/property, DB column and module names start with xxxt_
. (Here, "xxx" is "wbe" for "Web-based ERP". The "t" is "e" for core engine, "m" for "module" or "me" if used in both. I kept thinking all the "me" code was put in by an exceptionally egotistical developer whose previous language is Visual Basic.) For example, a "Hello World" module would be named "wbem_hello_world". The code-containing file would be "wbem_hello_world.php", with a wbem_hello_world
class. If you want to name a field, don't just name the property; it must follow the convention of wbem_class_name[wbem_form_name][fields][wbem_field_name]
(where field_name
is the name of the corresponding field in the DB, because tight coupling is important). I think the idea was to use some sort of ORM, but I don't see any sign of that actually happening. I yearn for Dapper.
4. To carry on from the previous point about a hypothetical new module ... The class would contain a wbem_hello_world_setup()
method (which contains nothing more than instructions on how to generate configuration and meta information as XML, something that could be a few lines of text in a static .cfg
, .info
, .ini
or even .json
file) and the greeting method would be named wbem_hello_user($wbem_users_name)
. It would contain the single line $global->wbe_print_response('<p>Hello, '. $wbe_users_name . Welcome to Web-based ERP.'</p>');
(itself a wrapper that calls echo()
). I think you get an idea of how bloated, unwieldy and verbose the code is, most of which could be easily avoided. (If I want to find out what a module's intended purpose is or how to use it, I actually have to read the setup code, instead of a README file. There's no guarantee that the setup method is at the beginning of the file, either. Again, this is something that Drupal got right. Despite it seeming inconvenient to create and update two or more separate files for a simple module, there's a clear separation of concerns evident there: Configuration, dependency and meta data/information isn't tied up with execution logic, since it changes far less often and is parsed/required far less often.)
5. All bridging tables have _link
at the end of their names, on the off chance that it's not apparent that they are such. They also contain columns for audit information (xxxt_created_BYuserID
, xxxt_date_created
, etc.), instead of in dedicated audit tables, on the off chance that someone just might want to run an audit on the system once in a blue moon.
6. Development documentation doesn't actually aid developers: I know the best documentation for a system is supposed to be the code itself, but that assumes that the code is sufficiently well-writ to be self-documenting (or at least contains usefully informative comments). However, it isn't that in this case. The documentation for it is equally opaque. For example, UIDs/primary keys in tables are always _id
(also non-standard) because they're not meant to be displayed to users (and hidden fields start with underscores). However, the documentation doesn't state that this applies to the DB, but only mentions it, in passing, when covering how to create classes (which, non-obviously must be tightly coupled to the underlying data storage and retrieval mechanism). It also doesn't contain anything about how to perform CRUD operations on the DB; I had to search for SQL statements in the code in order to figure out how that's done ($result_set = $global['wbeme_database']->wbeme_sqlQuery($xxxt_query_string);
and so on, in case you're interested). No PDO or prepared statement use is done here, of course. I wouldn't be surprised to find it's using the old and unsafe mysql_
functions, instead of mysqli_
`. If I get to a point where I've got the time to read all of the docs, make sense of them and make notes, figure out how to actually create a module/plugin, I'll build a dev_tools
module that has documented examples showing how to do various things in the code (as well as does some introspection so developers can get things like the names of tables (and their columns) in the DB without having to run these queries and print_r()
the result sets on the test system before actually writing queries or other experimental test code. It'll have to be called wbem_developer_tools
and have a `wbem_developer_tools.php
file define the wbem_developer_tools
class if it's going to follow the convention, bien sûr ...)
7. I spent at least two days reading up on git
and cleaning up the repo. After explaining to the project lead what I had read (including clearly-phrased standard practice), in multiple places (which I cited), he still insisted that text files should be committed/pushed to the repo with CRLF
line endings (despite the fact that git stores such files with LF
internally, changes any files that end CRLF
, will warn about the fact it does so and can/must not be configured to do otherwise, not to mention that even Windows notepad
supports LF
line endings now). I'd like to blame him/that for the whole-file diff issue, but that's not charitable/fair. At any rate, I have confidence in neither his abilities/skills as a developer nor his level of intelligence. I shouldn't be surprised, either, since the code on which I have worked shows evidence of developers ignoring common coding conventions too. I assume this is deliberate, since I've been instructed to not correct it (and even to put errors back in). Given the overuse of a Smurf entity naming convention, it seems odd that developers omit white space that makes their code more readable. To their credit, they are rather fond of parentheses.
8. The menu system is an ugly and cumbersome mess. For one thing, links in it are all down the side of the page, under a block for user information and edit/login/exit links (which, itself, should be a drop-down menu in the top-right corner). Instead of being sticky at the top, they're positioned relative to the main container content and can flow under the fold (instead of being scrollable). If I don't scroll the page to the exact optimal position or majorly reduce page zoom with <Ctrl>+<->
, it's quite easy for me to not be able to access the relevant menu item that I want. Additionally, the items aren't ordered alphabetically or logically grouped by category/functionality (both in the main menu and a few layers of submenus), as far as I can tell. There are way too many of them displayed at a time, which results in clutter. To be fair, this could happen to a Drupal site with plenty of mods installed, but it still did a better job of organising them into categories.
9. There are no "back" buttons or breadcrumb trails, helpful in the event that a user makes a mistake or clicks on the wrong thing. (It is all too easy to do so, especially since there's a noticeable lack of informative tool-tips and messages). Considering that the system primarily runs on $_GET[]
parameters in URLs, it should be relatively straightforward/trivial to build them (even if with JavaScript), not that anything about this system is straightforward/trivial to do. (When estimating time required to make a change or add a feature, I usually multiply my estimate by three for the worst-case scenario. For this system, I multiply by five. The lead developer seemed surprised I stated that I expect to be neither efficient nor productive when working on this code base and under the artificial constraints it imposes. I can't fathom why.)
10. img
elements containing text lack alt
attributes. This is particularly egregious practice when the images could easily be backgrounds for the actual text itself. There are probably a number of glaring cases where elements could benefit from aria
attributes, but I've not had time to look. To add insult to injury, most of the embedded text is in Comic Sans. (Why does anyone think this is acceptable for use anywhere other than on a card for/to someone below the age of thirteen?)
11. They have no coding standard or style guide in their documentation (over five hundred pages thereof, ~80% of it aimed at developers). However, it seems to be a case of reading the PHP FIG PSRs and doing the exact opposite of what those state must/should be done. For example: I spent over two hours putting trailing white space back into a file and resolving a merge conflict in git because of it, after the lead developer complained that my editor performed automatic code cleanup to remove it from blank lines. If there's a right way to do something, these folks will deliberately do it the wrong way for no discernibly valid reason. To make matters worse, their code "standards"/style guide document doesn't exist; it's a verbal agreement. (The fact that I explicitly asked where I can find it is probably the first time anyone bothered to write it down.) Seriously, WTAF?
How to: Use
git commit
as a weapon
They clearly like dicking around with developers who try to be professional and get actual work done, instead of spending time on no-brainer non-issues like following common conventions and standards. That's a two-way street, though, so fuck 'em. If they're going to waste my time, I'll happily waste theirs and charge them for it (such as writing and editing this during work hours). Don't mess with a honey badger, because a honey badger will fuck you up; it doesn't give a fuck!
No points will be awarded for how many code smells you can identify from what I've described above, but "speculative coding" (including things "in case we need them in the future") is one of them, especially since removing unused code (even if commented-out) from files thousands of lines long (and containing multiple classes) is definitely verboten, despite the fact it's stored in version control.
I still think "code smells" is a stupid name/metaphor for the phenomenon, but that's beside the point.
As a bonus, I found this horrible bit of redundant/repetitive code in the last file in/on which I worked (the code block editor is also playing silly buggers):
<?php
// ...
$wbem_default_data=array();$wbem_count++;
$wbem_default_data[$wbem_count]['name']='Cash';
$wbem_count++;
$wbem_default_data[$wbem_count]['name']='Credit-Card';
// On it goes for another eight names.
// Clearly, iterators are too difficult to code:$wbem_default_data = [];
$wbem_names = ["Cash", "Credit-Card", "..."];
foreach ($wbem_names as $n) {
/* Counting is probably not even strictly necessary;
* I'm not sure where $wbem_count is defined or
* why the offset by one. Maybe it's an error of
* copying and pasting or the item at index 0
* was removed. At any rate, it's ugly. */ $wbem_count++;
$wbem_default_data[$wbem_count]["name"] = $n;
}
I was planning to tack this onto the end of my previous post. Considering the size of both of them after writing this, I'm glad that I didn't. It would probably be a completely unreadable mess (as opposed to only partially), which seems apt in the circumstances.
Le sigh. By the time I've found a better job and called quits, who knows how many posts on the topic I'll have published? I suspect many, at this rate. Since I've already reached the limit of the number of blogs I can have for a single account here, I'm going to tag these posts "Web-based Derp" from now on. The sad thing is that even if I did have access to the core code and time to fix/redesign the system, I doubt I'd get paid to do so, rather than add more cruft to it. (Maybe if I learn how to use reflection in PHP, I might be able to get the system to write out the code it runs as it does so, perhaps even improve how modules are hooked and registered by the engine code. Maybe mining the stack trace logs is a good start. Maybe pigs will interbreed with ostriches and fly with their tails held forward. Frankly, I think I'd probably have more success poring over the core code for Drupal and WordPress.) If nothing else, these rantings may well serve as a useful guide on how to not write a customisable/extensible general-purposed framework/system, if I ever get to the point of building one.
For what it's worth, this wasn't intended to be a long-running comparison to Drupal as if Drupal's great (which it isn't, just better by comparison; still a low bar to clear). The things I do for money ...
I wish you happy coding, folks, as rare as that may be. I hope you're working on better projects (and for better teams) than I. Until my next post, Snark out! A developer's job is never really over; we're mostly in maintenance mode. Maybe I'll get some actuall work done today, but first lunch.
Thumbnail image: Photo by Cottonbro Studio on Pexels