Reposted from AdminExtra.com
So I looked into PHP errorlog then into its files and I saw… I dunno how to call it? Massive pile of dung?
It looks to me that vB5 code has been writen by guys who only just recently started learning PHP. No matter which file I open, I am suprised by some poor code that was written by amateour who had no idea, experience or knowledge of solution of problem at hand. IPB 2.0 that was released back in 2004 is years ahead of vBulletin 5 code.
Centralised input processing in vB5 is non-existant. Every sane application has some middleware between your application and request’s GET and POST variables… but this is not sane application. This is “write boilerplate(boilerplate($_REQUEST[‘meh’]), boilerplate, boilerplate::boilerplate(boilerplate->boilerplate())) every time you process request” type of application.
Their way of calling model?
$response = $api->callApi('content_attach', 'upload', array('file' => $_FILES['file']));
What about $api->get(‘content_attach’)->upload($_FILES[‘file’])? Oh… right, I forgot that knowledge of PHP and design patterns.
Dependency Injection?
$api = Api_InterfaceAbstract::instance();
$config = vB5_Config::instance();
How about service container?
phpDoc, php standard for documenting code by comments?
/**Upload a photo. Return an edit block and the photo URL.
*
**/
Notexistant.
Unified coding standard?
/**Upload a photo. Return an edit block and the photo URL.
*
**/
…
/** This method uploads an image and sets it as the logo in one step **/
…
/**
* @static
* When current lang charset isn’t the one in http content_type header
* this method will convert All Ajax $_POST data into current language charset
*/
None. *(and btw, last one is not correct phpDoc method documentation)
Tons of testing code commented out?
//echo "Logged into facebook
\n";
…
//$returnValue = array(‘error’ => ‘Invalid AJAX method called’);
…
// $page[‘searchJSON’] = $info[‘searchJSON’];
// $page[‘searchJSONStructure’] = $info[‘searchJSONStructure’];
// $page[‘error’] = $info[‘searchJSONStructure’][‘error’];
Present!
Routing-aware links builder?
header('Location: ' . vB5_Config::instance()->baseurl . '/register');
And to think Django is seven years old now and has this feature since beginning.
Separation between business logic and presentation?
if ($storecssasfile)
{
foreach($this->pending as $css)
{
$replace .= '\n";
}
}
else
{
$joinChar = (strpos($vbcsspath, '?') === false) ? '?' : '&';
$replace .= 'pending)) . "{$joinChar}ts=$cssdate \" />\n";
}
This code could be allright if it was html version aware… which its not, it generates XHTML.
Lots of evals?
eval(standard_error(fetch_error('searchnoresults', $displayCommon)));
…
eval(‘$faq[\’text\’] = “‘ . replace_template_variables($text) . ‘”;’);
Lots’a evals.
Modern object-oriented code?
(This one would require me to overstep bounds of quote to prove to those who dont have access to vBulletin code so you have to give me credit of trust.)
vBulletin 5 codebase is neither modern or object-oriented. Best way to descibe this code is to call it php 4 without reference assigment (“&=”) operator. Code in “include” directory is object-oriented (it even follows PSR-0 standard!), however majority of it looks more like set of functions that were put together in single container for easier access via autoloading than real objects. On opposide side of spectrum you have “core” directory which is procedural. Why? Who the hell writes “MVC” code (I am generous here) then goes “screw it, we will keep busines logic in procedural files”?
From reading their code I am getting idea that it was writen by people who adressed programming issues by mimicking other people code. They wanted “smart” way of handling data manipulation so they propably took idea from IPB’s 2.3 API’s. They wanted smart autoloading so they propably looked up xF’s (and by proxy Zend Framework) PSR-0 implementation. They wanted to write OOP code, but they didnt know what that really means so they packed their functions together in classess. They didnt had time to learn of flaws in PHP string manipulation functions which would force them to write input middleware and sanitize it for stuff like null bytes. They didnt know php5 has mb_string as standard lib for working on multibyte strings to they writed their own vb5_String class. Everybody has Auth class but they couldnt understand why one uses auth class in code so they made their own vb5_Auth which does nothing besides setting cookies on user’s sign-in and returning redirect link after successful sign-in. There is no logic in this class.
I see no greater scheme of things in vBulletin 5. By looks of code I can say that it was writen by group of programmers who had no workflow organisation, project plan and very different (usually low) skill levels. Lots of code was writen in hopes for best and without any backup plan. In many places they are using $_REQUEST variables without checking if those exist. If they do, its great. If not… well, depending on your php.ini and error_reporting PHP wont complain… unless its to php error log which eventually will be flooded with interpreter notices.
If I was to use vBulletin 5 code as benchmark of IB capabilities, I would make sure this company never gets any money from me and would stay off from their products. Internet Brands is network counterpart of ’96s Bethesda software (Google Buggerfall if you care). I see no care about product quality in IB work, only rush to people wallets packed in corporate marketing.
Finally this code also makes IB claims that xF “stands on the shoulders of more than a decade of development by Jelsoft” laughtable. Decade of development and best you could do is this pile of crap? You really have balls to rip people off like that.
Really, vBulletin 5 feels like aftertime project of some amateur who was using old PHP tutorials as source of knowledge, NOT corporation-backed product aimed to compete on market… and this is happening in 2012,l five years after Zend Framework 1.0 was made avaiable to the public.
What a disaster, I cant even find a words to describe this situation.
rafio