How can I improve my PHP model for my HTML5/JS mobile app?

I am posting the a section of the model of my application.

Any/all feedback would be very helpful as I am trying to build my PHP skills, but I will ask a couple specific questions that might help others as well.

-
Is my escaping, as is, effective enough?

-
I tried to separate my model into a file that will deal with functions exclusive to 'Groups' and a file that will deal with database functions used by all sections of the app. Is this a good strategy?

-
My Group model could use some more refactoring, but what do you think of the way I take and return data? (by using $data and $input, $data being the variable that is returned the controller and will be eventually sent to the view via JSON)

-
What are your thoughts about 'public function show_one($user_id, $group_id, $privacy)' which is meant to display one groups information, given proper privacy settings?

-
Any other input will be greatly appreciated. I am working hard to improve my programming skills everyday.

Group Model

2) The Group Model:

```
0,
'msg' =>'There was a small problem, try again soon.',
'data_type' => 'group',
'action' => '',
'results'=> array(),
);
//anything that will be put into the DB will be held in input
public $input = array();
public $exclude =array();

public function __construct($a) {
Global $db;
$db->escape($a);
$this->input = $db->escape($a);
@$this->data['action'] = $a['action'];
}
//move insert and data return up here

}

class create_group_model extends model {

public function insert_new_group($a) {
Global $db;
$b = $db->insert_array('group', $a, 'action');

if ($b['mysql_affected_rows']===1) {
$this->data['success'] = 1;
$this->data['msg'] = 'Congrats, you created a new group.';
array_push($this->data['results'], $b['mysql_insert_id']);
return $this->data;
} else {
$t

This is better, still a bit long, but unavoidably so. I might reiterate a few things from my last answer, but it is only because I'm too lazy to read it again to make sure I'm not. Sounds kind of ironic, but I do find it easier to just write it, rather than read it :)

General Overview

First let's have a quick look at your before diving in. I count four model classes, and what appears to be a fifth, which I will ignore for now. Hopefully each class is in its own file and not saved all into one. If not, you might want to strongly consider it. None of those four classes resembles a "true" model. Instead what you have here appear to be three model-controllers. Esentially, that means these are models whose functionality is designed for its controller rather than a controller whose functionality is designed for its model. The reason this is bad is because it leaves room for repetition. A final model should not be broken up into multiple parts, it should be one functional whole. That's not to say that you can't have a base class and extend subclasses to get to that whole, just that you shouldn't need to call three separate models to accomplish the task of what one model can do.

Now, A Little More In Depth

The very first thing I see is a global. Globals are evil. Pretend you never saw them. You will NEVER need them. I repeat NEVER. Especially not in a class. That's what properties are for.

class model {
    public $db;//should actually be private or protected

    public function __construct() {
        $this->db = new MySQLDatabase();
    }
}

Now every class that extends the model can use the same $db instance, and since it is a public property, you can also use that instance outside of the class once the model has been initialized. If you have an instance created before you initialize the model, and you want to use that one instead, then you should pass that instance to the constructor and set it as your property. The same holds true for regular functions as well: Pass it in, then return the modified version. However, this is not always the best solution. Sometimes, you can extend a class to accomplish the same thing.

//Using an existing instance
public function __construct( $db ) {
    $this->db = $db;
}
//Extending
class model extends MySQLDatabase() {
    //keep your current properties, they are fine
    public function __construct( $query ) {
        $this->input = $this->escape( $query );
        $this->data[ 'action' ] = isset( $a[ 'action' ] ) ? $a[ 'action' ] : NULL;
    }
}

Let's look at what we did here. The first thing I did was rename your parameter. Don't use vague names like $a. What is $a? I had to go all the way to the escape() method just to find out. Don't make your readers, or yourself, have to figure out what something is. Be descriptive with your variables/properties/functions/methods. Not too descriptive, but descriptive enough that we can work with what is given. This is called self-documenting code and makes your code much easier to read and debug. Next, I removed that redundant escape(). That method only ever returns a value, therefore declaring it without setting it to some variable or property is pointless. Lastly, I removed that error suppression. Don't use error suppression in your code. It is a sign of bad coding and should be avoided. The only exception might be while debugging, but then it should promptly be removed. What I've demonstrated above is a ternary statement and should fix those warnings you were occasionally getting about "action" being undefined in $a. Its the same as an if/else statement (if == ?, else == :), only shorter. Maybe you've seen it before, maybe not. Either way, be careful using them as they are not always well liked because they can cause issues with legibility if used incorrectly. My rule of thumb is, no nesting, complex, or long ternary; as long as it's short and sweet it's just fine.

Let me just say that this is not the implementation I am suggesting, I am just using this for demonstration. If just scanning the structure of these classes, as I did in the general overview, then the class and method names might look convincingly like a model's, but upon closer examination we find that it is actually a controller class. This would explain why I thought your models resembled model-controllers. And of course, a controller should not extend a model, of which your MySQLDatabase class is. As such, that very first example I showed you is the better choice here. Create a $db property and instantiate it in the constructor. It shouldn't be passed as an already existing instance because one shouldn't exist. It's the controller's job to instantiate the model.

Now that we know that your first four classes are actually controllers, we can better understand how they should work together. I'm going to leave the structure of these classes alone for now, because that will make this answer quite a bit longer, but you should take a

StackExchange Code Review Q#15400, answer score: 2