Comparing multiple fields from two tables, looking for matches, updating 2nd table

In my never-ending quest to improve my coding skills I am requesting peer input on the following code logic. I am asking because I'm -NOT- an expert, but would like to be one day. Source material and recommended read is greatly welcome (no, do NOT point me to the PHP manual) especially for code formatting, unit testing, etc.

Notes:

-
Layer separation is not important; i.e.: presentation and database connection logic was not abstracted to different classes. This is an admin usage only script. Sporadic usage, saw once a month.

-
I have just learned about dependence injection; was attempting to use it properly.

-
PHP version is 5.3.3. so fancy stuff like mysqli::query_all(ASSOC) is not an option.

Please let me know, in a constructive way please, how I could improve my skill at this trade.

```
status("createDBObject missing params", "ERROR", 1);
}

//Connect me to the SugarCRM database
$mysqliConn = null;
try
{
$mysqliConn = new mysqli($location, $user, $password, $database);
} catch (Exception $e) {
$this->status('Could not create DB object', $e, 1);
}

//if connection fails
if ( $mysqliConn->connect_error )
{
DependencyContainer::status( 'Could not connect to host', $mysqliConn->connect_error, 1 );
}else{
return $mysqliConn;
}
}

/**
* @Comment: Debug function call./
* @Param: title string, response string, die int
*/
public static function status( $title = null, $response = null, $die = 0 )
{
//no debug for you!
if( DEBUG == false )
return;

$isBrwsr = true;
//determine if we are in a browser or CLI
//spefcial thanks to: http://www.codediesel.com/php/quick-way-to-determine-if-php-is-running-at-the-command-line/
if(php_sapi_name() == 'cli' && empty($_SERVER['REMOTE_ADDR'])) {
$isBrwsr = false;
}

//easy full stop AND if debug is off and an error occures.
if($title == null)
$this->dieRequest("An error has oc

Well, first suggestion, don't use set_time_limit(). Try and refactor your code so that it is unnecessary, unless this is a CRON job or something and is running in the background. If you access it via a webpage then even if you are expecting it to take a while, it could be confusing. For instance, if you call this script via AJAX, then you will not see that it is loading, it will just sit there appearing to be doing nothing. I ran into a similar problem with a couple of my AJAX scripts recently and had to figure out how to separate the results so it didn't take as long. Additionally, be careful with that ignore_user_abort() function as it will require you to have to restart your server should you ever accidentally trigger an infinite loop.

You got your constants confused. define() is used to create a constant outside of a class. const is used to define a constant inside of a class.

No need to explicitly declare @Comment, just start typing. Your IDE should know that its a doccomment and act accordingly. @Comment isn't even a PHPDoc tag. See this link for a list of available tags and their uses. Its also important to note that if you give everything a decently descriptive name, doccomments like this are unnecessary. They should really only be used to point out the caveats and anything you think wont be obvious just by looking at the code.

When using @param you should only give one parameter. Declare new @params for each new parameter, otherwise you won't get the proper highlighting. Also, you have your parameter type and name switched.

/**
 * @param String $location etc...
 * @param String $user etc...
 etc...
 */

What is the point of all of these static methods? This completely defies the point of even having a class. You mentioned suggested reading, here's a wikipedia entry for OOP. I would suggest reading the whole thing, but that first paragraph is the important one here. In it it points out the key concepts of OOP. I would also suggest following the links to each of those concepts for a better understanding. Once you have done this, come back and look at your code and ask yourself if you are following most of these concepts. You don't have to follow every one of them, but if you aren't at least following most, then you have not justified OOP. In short, you probably wont need static very much at all. I've been doing this for quite a while and everything I have started to do with static I ended up scraping or turning into a helper function. Speaking of which, don't be shy about using helper functions either. If it doesn't fit in a class don't try and force it, helper functions are still useful, even in an OOP context.

You already have doccomments, why the inner comments? If you really need to document something about the method, do so in the doccomments at the beginning. That's what they are there for. Leave the inside of your methods comment free so that they don't get cluttered. Believe it or not, this really helps when trying to read the code, you wouldn't believe the difference it would make, especially if you auto-collapse doccomments in your IDE.

You should either use absolute equality === comparisons when checking for NULL, or the is_null() function. Personally, I prefer the later approach, but there is no real difference between the two, see this post for more. Unless, of course, you are not just checking for NULL, but for any FALSE value, in which case you can just do the following.

if( ! $location || ! $user || ! $database ) {

I'm actually kind of surprised this works at all... $this is not static and therefore can not be called in a static context. Use self:: instead. See this tutorial for details. I see you did this, or something very similar towards the end of the method, but the non-static implementation is still prevalent in the rest of your code and is wrong. Of course, removing the static keyword would be ideal and would make this unnecessary.

First, before going on, let me just say that I am seeing quite a few inconsistencies in this code: spaces between parenthesis, no spaces; switching back and forth between single and double quotes; $this-> vs. DependencyContainer:: or self::, as I mentioned above. This leads me to believe you either copy-pasted this together, have multiple people working on it, or have changed your style and not retroactively gone back to change everything. If one of the last two, strive to at least make each file consistent. If its too much work to go back and change everything, adjust your current style to fit. Or if you are working in a group, have a meeting and discuss code style and make everyone adhere to it. If its the first one, don't ever do that. Always type out any code you borrow. Doing so may help you to better understand it, or at the very least will ensure that its not obvious you stole it :)

But, the most heinous of all, IMO, are these braceless statements, especially when you use braced single st

StackExchange Code Review Q#15960, answer score: 4