checking for static var in child-of-child class?

[sneakyimp]

I've got a base class, let's call it DataObject. It's a foundation for all classes I use to interact with my database tables:

class DataObject {
  public function update_db_record($db) {
    $this->validate();
  }
  public function create_db_record($db) {
    $this->validate();
  }
  public function validate() {
    // i'm trying to flesh out this code!
  }
}

I have a script that automatically generates one class that inherits from this class for each table:

class DB_some_table {
  // lots of cool handy stuff here
}

Because these classes are auto-generated (and very convenient, I might add!) I don't want to have anyone modifying them.

Devs can extend those DB classes if they want to add extra functionality or specify a field_validation array that the base class, DataObject, will use to validate the data.

class DB_some_table_x extends DB_some_table {

// Define rules validate all fields posted for both client and server side validation
public static $field_validation
	= array(
		DB_some-table::COL_X      => array(
			"validateMethod" => "positive_int",
			"required"       => false
		),
		DB_some-table::COL_Y         => array(
			"validateMethod" => "phone_type",
			"required"       => true
		),
		DB_some-table::COL_Z => array(
			"validateMethod" => "phone",
			"required"       => true
		),
	);
}

What I'm wondering is what's the best way to refer to the static var defined in DB_some_table_x in my DataObjec::validate() method? This is working:

	public function validate() {
		if (isset($this::$field_validation)){
			// exists, we can use it to validate!
		} else {
			// doesn't exist, skip validation
		}
	}

Am I correct in thinking that $this in that context will always refer to the instantiated object? Might I want to try and use static or self or something instead?

[Weedpacket]

Declare the $field_validation array in DataObject (initialised to an empty array), with subclasses overriding it with their own declarations? DataObject wouldn't have to sweat over whether it's there or not. The array would be referenced with the static:: prefix.

class top
{
	protected static $foo = 'TOP';

function get()
{
	return static::$foo;
}

}

class bar extends top
{
	protected static $foo = 'BAR';
}


$t = new bar;

echo $t->get();

Or am I off base, here?

[sneakyimp]

Weedpacket;11055273 wrote:

Declare the $field_validation array in DataObject (initialised to an empty array), with subclasses overriding it with their own declarations?

Seems rather cluttered to have those never-used var declarations around.

Weedpacket;11055273 wrote:

DataObject wouldn't have to sweat over whether it's there or not. The array would be referenced with the static:: prefix.

I think checking whether it's there or not is probably the correct approach. I want to say this is "semantically correct" but am not sure exactly what that phrase means in coding. The static var $field_validation is only optionally defined if a coder wants any automated validation when creating or updating db records. It therefore makes sense to check for it, no? The code you offer is pretty clean but I'm not sure it communicates the optional nature of this variable. On the other hand, the presence of this variable in the base class may have advantages:
It might show up in autocomplete in my IDE
Its presence in the base class is a reminder that this functionality exists. Some developer new to my project might not be aware that they can define this variable unless there's one loitering around in the base class with a nice, informative JavaDoc comment.
* I may end up auto-generating $field_validation arrays based on database table definitions in the mid-level class. E.g., some validation that checks for valid ENUM values or maximum lengths or something like that.

I guess my question was more about the use of $this (which seems really awkward for a static var reference) versus the use of self or static or CLASS or ReflectionClass or whatever. I'm currently choking down the concept of late static bindings.

[Derokorian]

sneakyimp;11055293 wrote:

Seems rather cluttered to have those never-used var declarations around.

I think checking whether it's there or not is probably the correct approach. I want to say this is "semantically correct" but am not sure exactly what that phrase means in coding. The static var $field_validation is only optionally defined if a coder wants any automated validation when creating or updating db records. It therefore makes sense to check for it, no? The code you offer is pretty clean but I'm not sure it communicates the optional nature of this variable. On the other hand, the presence of this variable in the base class may have advantages:
It might show up in autocomplete in my IDE
Its presence in the base class is a reminder that this functionality exists. Some developer new to my project might not be aware that they can define this variable unless there's one loitering around in the base class with a nice, informative JavaDoc comment.
* I may end up auto-generating $field_validation arrays based on database table definitions in the mid-level class. E.g., some validation that checks for valid ENUM values or maximum lengths or something like that.

I guess my question was more about the use of $this (which seems really awkward for a static var reference) versus the use of self or static or CLASS or ReflectionClass or whatever. I'm currently choking down the concept of late static bindings.

In late static binding "static" refers to the current class in the hierarchy, "self" always refers to the class its defined in. This should show you how it works: https://3v4l.org/vVkii

Additionally, I would never rely on something that may or may not be defined in a child. Instead, I would define it explicitly in the class that uses it, even if its empty. This way its harder to accidentally type it wrong when adding it to the child class. See my BaseModel for example -> https://github.com/derokorian/DeroFramework/blob/master/src/Data/BaseModel.php Here you can see how I define constants TABLE_NAME, COLUMNS, SUB_OBJECTS but they are empty, to be filled in by extending classes. Other benefits include a class definition that relies only on things that are part of the class definition.

[sneakyimp]

Derokorian;11055305 wrote:

In late static binding "static" refers to the current class in the hierarchy, "self" always refers to the class its defined in. This should show you how it works: https://3v4l.org/vVkii

Thanks for great example. I feel like I knew this once upon a time. I don't recall ever having used static or self in conjunction with a static variable.

Derokorian;11055305 wrote:

Additionally, I would never rely on something that may or may not be defined in a child. Instead, I would define it explicitly in the class that uses it, even if its empty. This way its harder to accidentally type it wrong when adding it to the child class. See my BaseModel for example -> https://github.com/derokorian/DeroFramework/blob/master/src/Data/BaseModel.php Here you can see how I define constants TABLE_NAME, COLUMNS, SUB_OBJECTS but they are empty, to be filled in by extending classes. Other benefits include a class definition that relies only on things that are part of the class definition.

I'm guessing you mean that if we put the static var in the base class with an empty value (e.g., NULL) then our IDE's auto complete functionality will prevent mistyping the var name. I can totally get behind that. In my case the var is optional. If the user bothers to define any non-empty value, it should be an array that specifies validation to be performed on the object before date create/update.

[Derokorian]

sneakyimp;11055313 wrote:

I'm guessing you mean that if we put the static var in the base class with an empty value (e.g., NULL) then our IDE's auto complete functionality will prevent mistyping the var name. I can totally get behind that. In my case the var is optional. If the user bothers to define any non-empty value, it should be an array that specifies validation to be performed on the object before date create/update.

Right, but my point was more along the lines of you shouldn't have any code dependent on a member of the class that isn't defined as part of that class. If something is optionally defined in a child class, and the functionality that works with it is in the parent class, then the parent class should explicitly define it and to the type it should be (Like in my BaseModel how SUB_OBJECTS is an array, but most of the models extending it don't ever redefine that or change it, but if they do the parent provides functionality for it). The problem with not defining it in the parent is much more than losing auto-complete in an IDE, its also that now you are defining a contract that's hidden down in method bodies. I should be able to look at the vars of the class I'm extending, to know what I can do with it.

[sneakyimp]

Derokorian;11055317 wrote:

Right, but my point was more along the lines of you shouldn't have any code dependent on a member of the class that isn't defined as part of that class. If something is optionally defined in a child class, and the functionality that works with it is in the parent class, then the parent class should explicitly define it and to the type it should be

Hmmm. I think I might disagree for a few reasons:
My base class is an abstract class and is therefore never instantiated.
Defining these never-used public static vars in the base abstract class only adds more unnecessary (and possibly confusing) lines of code. It's sort of like seeing "this page intentionally left blank" in a user manual.
Because the var is static, we'd have to refer to it either using TheClassName::$field_validation or self::$field_validation or static::$field_validation. Given that I'm writing a validation routine in the abstract base class, I cannot use either of the first two options.
I can think of any situation where I would ever set this variable using TheClassName::$field_validation so the IDE/autocomplete angle is probably unlikely to happen. It might happen, but I don't think it's very likely at all in this project.

Derokorian;11055317 wrote:

The problem with not defining it in the parent is much more than losing auto-complete in an IDE, its also that now you are defining a contract that's hidden down in method bodies. I should be able to look at the vars of the class I'm extending, to know what I can do with it.

I tend to agree with this line of reasoning. One of the things that worries me about my scheme here is that a user can instantiate either DB_some_table or DB_some_table_x. If they instantiate the former, then static::$field_validation will be empty so no validation is performed. If they instantiate the latter, they'll get the automatic validation if they bother to define the correct public static $field_validation array in that class. I'm leaning toward making DB_some_table an abstract class too and forcing every class to have a DB_whatever_x class or something like that.

[laserlight]

sneakyimp wrote:

* My base class is an abstract class and is therefore never instantiated.

I don't see how this is cause for disagreement. An abstract class specifies an interface, except that unlike an interface construct, it may also provide partial implementation. You have both here: the implementation of the validate method, and also the interface specified by having the $field_validation static member (though it is not the usual method-type interface).

sneakyimp wrote:

* Defining these never-used public static vars in the base abstract class only adds more unnecessary (and possibly confusing) lines of code. It's sort of like seeing "this page intentionally left blank" in a user manual.

Python has a special NotImplemented value that can be used for precisely this situation, and I have not found it unnecessary or confusing. Since PHP does not have such a special value, perhaps it might be confusing, but the confusion can be and probably should be removed with a comment.

sneakyimp wrote:

* Because the var is static, we'd have to refer to it either using TheClassName::$field_validation or self::$field_validation or static::$field_validation. Given that I'm writing a validation routine in the abstract base class, I cannot use either of the first two options.

But you can use the third option, and in the end all you need is at least one usable option, right?

sneakyimp wrote:

One of the things that worries me about my scheme here is that a user can instantiate either DB_some_table or DB_some_table_x. If they instantiate the former, then static::$field_validation will be empty so no validation is performed. If they instantiate the latter, they'll get the automatic validation if they bother to define the correct public static $field_validation array in that class.

If you think that no validation being performed is necessarily a Bad Thing, then the validation routine could assert that static::$field_validation is not null.

[sneakyimp]

I've also run across another issue. I hope it's OK to air it in this thread. I can think of a way to solve it but would appreciate opinions.

As I described above, I have my base class, DataObject, which is actually an abstract class with one or two properties and one or two static vars. I automatically generate a whole bunch of classes that inherit from this class -- one for each table in the database, e.g., DB_some_table. I have a problem where if I assign a bogus property to an instance of DB_some_table -- i.e., "bogus" meaning that it doesn't correspond to a column in some_table -- then the SQL that gets generated results in an error because the query builder will generate SQL that refers to this non-existent db column.

I need some way to efficiently distinguish which properties in this inheritance hierarchy correspond to my db columns and which do not. This ability to distinguish comes into play in two important places:
when instantiating my DB_some_table object from an array -- the array might have additional values that we don't want. We just want to extract from the array those keys that correspond to db column names. E.g., imagine we
when generating SQL from the object

There is a one-to-one correspondence between the columns of some_table in my DB and the public properties of the automatically-generated class, DB_some_table. E.g., DB_some_table might look like this:

class DB_some_table extends DataObject {
  // each of these properties corresponds to a colum in some_table
  public $dbcol_foo;
  public $dbcol_bar;
  public $dbcol_ruby_on_rails_can_suck_it;
}

I need some way that code in the DataObject class can distinguish its own properties from those of DB_some_table efficiently. I've been looking into [man]ReflectionClass[/man] but haven't yet found a way to distinguish these dbcol properties from those of DataObject and also distinguish them from any properties which may be found in a class that extends DB_some_table and also distinguish these dbcol properties from any static variables that are hanging around. Furthermore, I don't want to be running a bunch or reflection code and looping every time I try and set a property or instantiate an instance of this class (there could be dozens) or update a record. I'm leaning toward generating a static var in DB_some_table which is an array of all the dbcol names but it seems redundant:

class DB_some_table extends DataObject {
  // each of these properties corresponds to a colum in some_table
  public $dbcol_foo;
  public $dbcol_bar;
  public $dbcol_ruby_on_rails_can_suck_it;

  public static valid_dbcol_props = array("dbcol_foo", "dbcol_bar", "dbcol_ruby_on_rails_can_suck_it");
}

Here's a brief illustration of the problem where I assign bogus properties,

// fetch some object from the db by its primary key
$p = DB_some_table::fetch_by_pkey($this->db, 123); // returns a DB_some_table object
$p->blarg = "foobar"; // set some property not defined in the class which doesn't correspond to any column in some_table
$p->update_db_record($this->db); // use the CodeIgniter object to update the database, this will throw exception

The Exception would be something like

DB Update failed: error code=1054 ; error message=Unknown column 'blarg' in 'field list'

As you might suspect, this error is handed down by MySQL when it attempts to execute the SQL generated by the CodeIgniter query builder.

I would very much like to throw an exception when someone tries to set some bogus property ("blarg" in my example). I believe I can accomplish this by defining a set method in my DataObject base class but this method needs to be able to efficiently distinguish db col properties from others. I've tried a few things. E.g., defining a set method in the DataObject base class:

public function __set($prop, $value) {
  $rc = new ReflectionClass(static::class); // or get_called_class() or whatever
  $props = $rc->getProperties(ReflectionProperty::IS_PUBLIC); // returns both static properties and public properties of any extending class, you *can* loop to eliminate static
  $props = $rc->getDefaultProperties(); // returns both static properties of class and public properties of any extending class
  $props = get_object_vars($this); // eliminates static vars but includes properties of extending class.

  if (!in_array($prop, $props) {
    throw new Exception("Invalid property name");
  }

}

A couple of things to note about this:
set won't get called if user is assigning legitimate public properties so it seems pretty good for intercepting "bogus" property assignments
we don't want to do all that ReflectionClass rigamarole every single time we set some value so we'd need to maybe set some static var with the orthodox list of dbcol names
* if set is defined in DataObject, there's no foolproof way to identify the name of the correct class from the called class -- or at least none that I can think of.

[sneakyimp]

laserlight;11055361 wrote:

I don't see how this is cause for disagreement...

I guess you're right. Sigh. Makes me sad to have code in there that doesn't seem to do anything. Also, defining NULL $field_validation arrays and then insisting that this value NOT be null seems a bit dogmatic.

[Derokorian]

sneakyimp;11055365 wrote:

when instantiating my DB_some_table object from an array -- the array might* have additional values that we don't want. We just want to extract from the array those keys that correspond to db column names. E.g., imagine we

Imagine we... what?? 😛

Take a look at my base contract, all it does is specify a fromArray static constructor:

abstract class Contract
{
    public static function fromArray(array $aProps) : Contract
    {
        $s = new static();
        foreach ($aProps as $k => $v) {

// Note here, that "static::class" may be replaced with get_called_class() if you're not on 5.5+
            if (property_exists(static::class, $k)) {
                $s->$k = $v;
            }
        }
        return $s;
    }
}

I feel like this would give you exactly what you need within the limitations specified.

sneakyimp;11055365 wrote:

* when generating SQL from the object

Again, using property_exists with the class name instead of the instance will allow you to distinguish between properties of the class, and dynamic properties of the object. See above example.

sneakyimp;11055365 wrote:

I need some way that code in the DataObject class can distinguish its own properties from those of DB_some_table efficiently. I've been looking into [man]ReflectionClass[/man] but haven't yet found a way to distinguish these dbcol properties from those of DataObject and also distinguish them from any properties which may be found in a class that extends DB_some_table and also distinguish these dbcol properties from any static variables that are hanging around. Furthermore, I don't want to be running a bunch or reflection code and looping every time I try and set a property or instantiate an instance of this class (there could be dozens) or update a record. I'm leaning toward generating a static var in DB_some_table which is an array of all the dbcol names but it seems redundant:

Here's a brief illustration of the problem where I assign bogus properties,

// fetch some object from the db by its primary key
$p = DB_some_table::fetch_by_pkey($this->db, 123); // returns a DB_some_table object
$p->blarg = "foobar"; // set some property not defined in the class which doesn't correspond to any column in some_table
$p->update_db_record($this->db); // use the CodeIgniter object to update the database, this will throw exception

The Exception would be something like

As you might suspect, this error is handed down by MySQL when it attempts to execute the SQL generated by the CodeIgniter query builder.

I would very much like to throw an exception when someone tries to set some bogus property ("blarg" in my example). I believe I can accomplish this by defining a set method in my DataObject base class but this method needs to be able to efficiently distinguish db col properties from others. I've tried a few things. E.g., defining a set method in the DataObject base class:

public function __set($prop, $value) {
  $rc = new ReflectionClass(static::class); // or get_called_class() or whatever
  $props = $rc->getProperties(ReflectionProperty::IS_PUBLIC); // returns both static properties and public properties of any extending class, you *can* loop to eliminate static
  $props = $rc->getDefaultProperties(); // returns both static properties of class and public properties of any extending class
  $props = get_object_vars($this); // eliminates static vars but includes properties of extending class.

  if (!in_array($prop, $props) {
    throw new Exception("Invalid property name");
  }

}

Again, here I believe using the class name with propert_exists is all you need

sneakyimp;11055365 wrote:

* if __set is defined in DataObject, there's no foolproof way to identify the name of the correct class from the called class -- or at least none that I can think of.

static::class or get_called_class depending on your php version is pretty fool proof.

[Derokorian]

sneakyimp;11055357 wrote:

Hmmm. I think I might disagree for a few reasons:
My base class is an abstract class and is therefore never instantiated.
Defining these never-used public static vars in the base abstract class only adds more unnecessary (and possibly confusing) lines of code. It's sort of like seeing "this page intentionally left blank" in a user manual.
Because the var is static, we'd have to refer to it either using TheClassName::$field_validation or self::$field_validation or static::$field_validation. Given that I'm writing a validation routine in the abstract base class, I cannot use either of the first two options.
I can think of any situation where I would ever set this variable using TheClassName::$field_validation so the IDE/autocomplete angle is probably unlikely to happen. It might happen, but I don't think it's very likely at all in this project.

Guess we'll just agree to disagree here. Auto-complete (at least in phpstorm) works when defining a property on a class, that exists an ancestor or interface being implemented. For example, this image shows me starting to define the sub_objects part of a class extending my BaseModel (previously linked).

sneakyimp;11055357 wrote:

I tend to agree with this line of reasoning. One of the things that worries me about my scheme here is that a user can instantiate either DB_some_table or DB_some_table_x. If they instantiate the former, then static::$field_validation will be empty so no validation is performed. If they instantiate the latter, they'll get the automatic validation if they bother to define the correct public static $field_validation array in that class. I'm leaning toward making DB_some_table an abstract class too and forcing every class to have a DB_whatever_x class or something like that.

Two options: a) add validation field to DB_some_table instead of extending it to define a single property or b) make this class abstract as you mentioned.

sneakyimp;11055367 wrote:

I guess you're right. Sigh. Makes me sad to have code in there that doesn't seem to do anything. Also, defining NULL $field_validation arrays and then insisting that this value NOT be null seems a bit dogmatic.

I would not set it to null, I would set it to the type you expect, in this case an array. Again, looking at my BaseModel you can see how I made TABLE_NAME a string, and COLUMNS & SUB_OBJECTS are arrays.

[sneakyimp]

Derokorian;11055371 wrote:

Imagine we... what?? 😛

Imagine you were reading what I meant to type!

But seriously, imagine the input array comes from input or some evil/careless programmer and might not be trustworthy.

Derokorian;11055371 wrote:

Take a look at my base contract, all it does is specify a fromArray static constructor...I feel like this would give you exactly what you need within the limitations specified.

My constructor looks suspiciously like your fromArray method and I still believe problems exist. Check out this proof-of-concept script to illustrate what I mean:

<?php
// base class, like derokorian's Contract
abstract class DataObject {

// none of these vars are in my db table
private $not_db_1;
protected $not_db_2;
public $not_db_3;

private static $static_1;
protected static $static_2;
public static $static_3;

public function __construct($arr=array())
{
	foreach ($arr as $key => $value) {
		if (property_exists($this, $key)) {
			echo "assigning $key\n";
			$this->$key = $value;
		} else {
			echo "skipping $key\n";
		}
	}
} // __construct()}
} // class DataObject

// each db table would have one of these
// which are auto-generated from the db
class DB_some_table extends DataObject{
	public $db_col_1;
	public $db_col_2;
	public $db_col_3;
}

// custom classes that extend each auto-generated class
// can add $field_validation or other orthodoxy which 
// cannot be reliably auto-generated from the DB table
class DB_some_table_x extends DB_some_table {

// none of these vars in my db table
private $not_db_4;
protected $not_db_5;
public $not_db_6;

private static $static_4;
protected static $static_5;
public static $static_6;

}

// for the sake of testing. let's assume a bad guy or
// stupid programmer tries to pull some shenanigans
$obj = new DB_some_table_x(array(
	"not_db_1" => "BAD",
	"not_db_2" => "BAD",
	"not_db_3" => "BAD",
//	"not_db_4" => "BAD", // PHP Fatal error:  Cannot access private property DB_some_table_x::$not_db_4
	"not_db_5" => "BAD",
	"not_db_6" => "BAD",
	"db_col_1" => "GOOD",
	"db_col_2" => "GOOD",
	"db_col_3" => "GOOD",
	"static_1" => "BAD",
	"static_2" => "BAD",
	"static_3" => "BAD",
//	"static_4" => "BAD", // PHP Fatal error:  Cannot access private property DB_some_table_x::$static_4
	"static_5" => "BAD",
	"static_6" => "BAD",
	"blarg" => "BAD"
));
var_dump($obj);

As you can see from the output of that script, it's very easy to sneak in "BAD" data using such a constructor. property_exists only skips a couple of items:

assigning not_db_1
assigning not_db_2
assigning not_db_3
assigning not_db_5
assigning not_db_6
assigning db_col_1
assigning db_col_2
assigning db_col_3
skipping static_1
assigning static_2
assigning static_3
assigning static_5
assigning static_6
skipping blarg
object(DB_some_table_x)#1 (13) {
  ["not_db_4":"DB_some_table_x":private]=>
  NULL
  ["not_db_5":protected]=>
  string(3) "BAD"
  ["not_db_6"]=>
  string(3) "BAD"
  ["db_col_1"]=>
  string(4) "GOOD"
  ["db_col_2"]=>
  string(4) "GOOD"
  ["db_col_3"]=>
  string(4) "GOOD"
  ["not_db_1":"DataObject":private]=>
  string(3) "BAD"
  ["not_db_2":protected]=>
  string(3) "BAD"
  ["not_db_3"]=>
  string(3) "BAD"
  ["static_2":protected]=>
  string(3) "BAD"
  ["static_3"]=>
  string(3) "BAD"
  ["static_5":protected]=>
  string(3) "BAD"
  ["static_6"]=>
  string(3) "BAD"
}

Derokorian;11055371 wrote:

Again, here I believe using the class name with propert_exists is all you need

Based on this example, I am pretty sure it's not enough. Weird to me that static vars are considered properties. Also problematic are the inability to distinguish properties of DB_some_table from its base class and extending class.

Derokorian;11055371 wrote:

static::class or get_called_class depending on your php version is pretty fool proof.

If I add a set method to my base DataObject class, property_exists is still not 100% useful in distinguishing base class and extending class properties (static or instance vars) from those of DB_some_table

	public function __set($prop, $value) {
		if (property_exists(static::class, $prop)) {
			echo "setting $prop\n";
			$this->$prop = $value;
		} else {
			echo "NOT setting $prop\n";
		}
	}

After adding DataObject::set, I append this code to the end:

$data2 = array(
	"not_db_1" => "BAD2",
	"not_db_2" => "BAD2",
	"not_db_3" => "BAD2",
	"not_db_4" => "BAD2", // PHP Fatal error:  Cannot access private property DB_some_table_x::$not_db_4
	"not_db_5" => "BAD2",
	"not_db_6" => "BAD2",
	"db_col_1" => "GOOD2",
	"db_col_2" => "GOOD2",
	"db_col_3" => "GOOD2",
	"static_1" => "BAD2",
	"static_2" => "BAD2",
	"static_3" => "BAD2",
	"static_4" => "BAD2", // PHP Fatal error:  Cannot access private property DB_some_table_x::$static_4
	"static_5" => "BAD2",
	"static_6" => "BAD2",
	"blarg" => "BAD2"
);
foreach($data2 as $key => $val) {
	$obj->$key = $val;
}
var_dump($obj);

The output shows the new __set method NOT SETTING private members of the base class (does this make sense???) and also properly rejecting blarg which has no place among these properties:

NOT setting not_db_1
setting not_db_2
setting not_db_4
setting not_db_5
NOT setting static_1
setting static_2
setting static_4
setting static_5
NOT setting blarg
object(DB_some_table_x)#1 (13) {
  ["not_db_4":"DB_some_table_x":private]=>
  NULL
  ["not_db_5":protected]=>
  string(4) "BAD2"
  ["not_db_6"]=>
  string(4) "BAD2"
  ["db_col_1"]=>
  string(5) "GOOD2"
  ["db_col_2"]=>
  string(5) "GOOD2"
  ["db_col_3"]=>
  string(5) "GOOD2"
  ["not_db_1":"DataObject":private]=>
  string(3) "BAD"
  ["not_db_2":protected]=>
  string(4) "BAD2"
  ["not_db_3"]=>
  string(4) "BAD2"
  ["static_2":protected]=>
  string(4) "BAD2"
  ["static_3"]=>
  string(4) "BAD2"
  ["static_5":protected]=>
  string(4) "BAD2"
  ["static_6"]=>
  string(4) "BAD2"
}

HOWEVER, property_exists tells us nothing about the private/protected/public status of the variable so all of our bad data gets assigned to our object. I think this should clearly illustrate problems with using property_exists.

[Derokorian]

I guess the question now becomes, why are you mixing your contracts with all kinds of other "not_in_db" things?? If you want your object to be a contract with the database table (something I don't agree with in the first place btw, classes should represent how the code needs things, not the database IMHO), then your class should only house that contract and nothing else. Taking for example, these 2 files exist in my site, App\Model\Blog\Model and App\Model\Blog\Post, the Model being what provides interactivity with a data store, and the Post being a contract that defines how I work with a Post in code. Now that contract does not map directly to a database table though... Instead its the full object I expect of a Post in code (with things like username, for example, which are retrieved via a JOIN on the users table, and body and rawBody, where rawBody is what's in the db, and the text area during editing, but body is what's shown when rendering). The model returns Post objects, or receives Post objects for creation or modification.

I think the disconnect in our discussion here, is how we design our classes generally speaking. In my opinion you are mixing many concerns into one class. A single class should only be responsible for one thing. Using my Model and Post above, the Post is responsible for simply defining our the object should be used in the code, the model is responsible for translating to/from data store format(s) and the Post code level contract. The data interface I inject into the Model is responsible for actually interacting with said data store, so my model doesn't even care what type of store it is (be it flat file, no sql, or rdbms, or some remote api).

As for your issue, you could alternatively use a Factory pattern. This way the method in charge of constructing the object has no access to private or protected properties? But it still doesn't solve your problem, as your example shows public properties as being bad to set... (which makes no sense to me btw!). So I guess without a repository of safe properties, there is no way to know - except for reflection of course, which you specifically don't want to use.

[sneakyimp]

Derokorian;11055383 wrote:

I guess the question now becomes, why are you mixing your contracts with all kinds of other "not_in_db" things??

The 'not in db things' do relate to the database, but are not the names of database columns. E.g, my DB_user_phone_x class, which extends DB_user_phone has some static vars:

	public static $phone_type_options = array(self::ENUM_PHONE_TYPE_HOME, self::ENUM_PHONE_TYPE_MOBILE, self::ENUM_PHONE_TYPE_WORK);


// Define methods to validate all fields posted for both client and server side validation
public static $field_validation
	= array(
		DB_vlnt_user_phone::COL_USER_ID      => array(
			"validateMethod" => "positive_int",
			"required"       => true
		),
		DB_vlnt_user_phone::COL_TYPE         => array(
			"validateMethod" => "phone_type",
			"required"       => true
		),
		DB_vlnt_user_phone::COL_PHONE_NUMBER => array(
			"validateMethod" => "phone",
			"required"       => true
		),

	);

As you can see from my last code example above, these static vars show up when using property_exists. It seemed natural to define these db-related concepts and validation in the extending class.

Derokorian;11055383 wrote:

If you want your object to be a contract with the database table (something I don't agree with in the first place btw, classes should represent how the code needs things, not the database IMHO), then your class should only house that contract and nothing else. Taking for example, these 2 files exist in my site, App\Model\Blog\Model and App\Model\Blog\Post, the Model being what provides interactivity with a data store, and the Post being a contract that defines how I work with a Post in code. Now that contract does not map directly to a database table though... Instead its the full object I expect of a Post in code (with things like username, for example, which are retrieved via a JOIN on the users table, and body and rawBody, where rawBody is what's in the db, and the text area during editing, but body is what's shown when rendering). The model returns Post objects, or receives Post objects for creation or modification.

Although I appreciate you sharing, I have admittedly not taken the time to digest your scheme in its entirety. I looked at a few classes and it seems very, very different than what I've constructed. In particular, I saw const COLUMNS and decided I did not want to emulate your approach. Declaring each of my database columns as a distinct public property in my DB_some_table class has proven to be of tremendous utility in writing code because of how these classes offer up autocompletion that helps one easily find db tables and their columns. It's hard to describe how much it has streamlined things -- the primary advantage being that I don't need to continually refer to a schema to remember table or column names from my DB -- they just show up in autocomplete as I type.

Derokorian;11055383 wrote:

I think the disconnect in our discussion here, is how we design our classes generally speaking. In my opinion you are mixing many concerns into one class.

Agreed that our approaches seem different, but not as different as you might think. In practice, I have very, very few not_db_x properties in these classes and the ones I have seem to me to be there for a pretty good reason. I offered the counterexamples above merely as a fairly comprehensive demonstration of why property_exists was not sufficient in my case.

Derokorian;11055383 wrote:

A single class should only be responsible for one thing.

I think this statement is rather impossible to translate into code guidelines in any meaningful way.

Derokorian;11055383 wrote:

Using my Model and Post above, the Post is responsible for simply defining our the object should be used in the code, the model is responsible for translating to/from data store format(s) and the Post code level contract. The data interface I inject into the Model is responsible for actually interacting with said data store, so my model doesn't even care what type of store it is (be it flat file, no sql, or rdbms, or some remote api).

I searched your project for the word post and didn't find anything meaningful to help me understand what you are suggesting. Aside from the fact you are using a lot of extremely recent PHP features (not a possibility for my project), your framework is a very different beast than my beast.

Derokorian;11055383 wrote:

As for your issue, you could alternatively use a Factory pattern. This way the method in charge of constructing the object has no access to private or protected properties? But it still doesn't solve your problem, as your example shows public properties as being bad to set... (which makes no sense to me btw!). So I guess without a repository of safe properties, there is no way to know - except for reflection of course, which you specifically don't want to use.

The last example I gave was merely to illustrate why property_exists might have problems. I don't expect to ever intentionally supply any not_db_x values to my DB_whatever constructors, but some other bonehead might. Part of my task here is to prevent abuse/mistakes by other developers on this project. Your suggestion of a Factory pattern is interesting. I also truly appreciate your efforts to refine my thinking. Given the constraints of this project, though, I hope that there might be some way to distinguish my properties that correspond to db columns (i.e., all public properties of the DB_some_table class) from any other private/protected/public/static/whatever properties that might exist due to inheritance or composition or whatever. The system has been working so far -- it's quite intuitive and is keeping my collaborators out of trouble. It just needs a little refinement.

EDIT: I'm not opposed to reflection either. It just seems like it would be really inefficient to by parsing ReflectionClass results in a __set method and I'm having trouble picturing how to effect either a caching or lazy loading scheme to store the results of reflection after it is initially performed.

[sneakyimp]

Ok having some luck.

I had forgotten that the magic method set is only called when a property is inaccessible:

php docs wrote:

The overloading methods are invoked when interacting with properties or methods that have not been declared or are not visible in the current scope.

that said, this appears to bounce non-existent property assignments pretty well:

 	public function __set($prop, $value) {
		throw new Exception_Data_Object($prop . " is not a valid property for " . static::class);
 	}

[Derokorian]

sneakyimp;11055385 wrote:

The 'not in db things' do relate to the database, but are not the names of database columns. E.g, my DB_user_phone_x class, which extends DB_user_phone has some static vars:

	public static $phone_type_options = array(self::ENUM_PHONE_TYPE_HOME, self::ENUM_PHONE_TYPE_MOBILE, self::ENUM_PHONE_TYPE_WORK);


// Define methods to validate all fields posted for both client and server side validation
public static $field_validation
	= array(
		DB_vlnt_user_phone::COL_USER_ID      => array(
			"validateMethod" => "positive_int",
			"required"       => true
		),
		DB_vlnt_user_phone::COL_TYPE         => array(
			"validateMethod" => "phone_type",
			"required"       => true
		),
		DB_vlnt_user_phone::COL_PHONE_NUMBER => array(
			"validateMethod" => "phone",
			"required"       => true
		),

	);

As you can see from my last code example above, these static vars show up when using property_exists. It seemed natural to define these db-related concepts and validation in the extending class.

Although I appreciate you sharing, I have admittedly not taken the time to digest your scheme in its entirety. I looked at a few classes and it seems very, very different than what I've constructed. In particular, I saw const COLUMNS and decided I did not want to emulate your approach. Declaring each of my database columns as a distinct public property in my DB_some_table class has proven to be of tremendous utility in writing code because of how these classes offer up autocompletion that helps one easily find db tables and their columns. It's hard to describe how much it has streamlined things -- the primary advantage being that I don't need to continually refer to a schema to remember table or column names from my DB -- they just show up in autocomplete as I type.

That's why I was saying i have a contract class that represents an object in code, and a model which ties that class to a data store. In a controller, I might do something like $blogModel->insert(Post::fromArray($_POST['post']));

sneakyimp;11055385 wrote:

Agreed that our approaches seem different, but not as different as you might think. In practice, I have very, very few not_db_x properties in these classes and the ones I have seem to me to be there for a pretty good reason. I offered the counterexamples above merely as a fairly comprehensive demonstration of why property_exists was not sufficient in my case.

Maybe you need to rethink how these properties are defined tho (for example, above you have a public static that will not change, sounds like that should be a const?).

sneakyimp;11055385 wrote:

I think this statement is rather impossible to translate into code guidelines in any meaningful way.

I agree that we disagree on this point then 🙂

sneakyimp;11055385 wrote:

I searched your project for the word post and didn't find anything meaningful to help me understand what you are suggesting. Aside from the fact you are using a lot of extremely recent PHP features (not a possibility for my project), your framework is a very different beast than my beast.

The project I was referring to, is my personal website, which utilizes my framework, but its not built into the framework. Here is my Post class: http://pastebin.com/yU8337g0 and the Model class: http://pastebin.com/PrX335hE

sneakyimp;11055385 wrote:

The last example I gave was merely to illustrate why property_exists might have problems. I don't expect to ever intentionally supply any not_db_x values to my DB_whatever constructors, but some other bonehead might. Part of my task here is to prevent abuse/mistakes by other developers on this project. Your suggestion of a Factory pattern is interesting. I also truly appreciate your efforts to refine my thinking. Given the constraints of this project, though, I hope that there might be some way to distinguish my properties that correspond to db columns (i.e., all public properties of the DB_some_table class) from any other private/protected/public/static/whatever properties that might exist due to inheritance or composition or whatever. The system has been working so far -- it's quite intuitive and is keeping my collaborators out of trouble. It just needs a little refinement.

EDIT: I'm not opposed to reflection either. It just seems like it would be really inefficient to by parsing ReflectionClass results in a __set method and I'm having trouble picturing how to effect either a caching or lazy loading scheme to store the results of reflection after it is initially performed.

Well assuming you can update the auto-generation of the classes, you can make the constructor private in dataobject, and then make a static constructor as part of the generated class. Something like:

public static function create($vars)
{
    $ref = new ReflectionClass(static::class);
    $props = array_map(function ($prop) {
        // returns just the property name
        $prop->name;
    }, array_filter($ref->getProperties(ReflectionProperty::IS_PUBLIC), function ($prop) {
        // removes static properties and anything further up the hierarchy
        return !$prop->isStatic() && $prop->class === static::class;
    });

$obj = new static();
foreach ($vars as $k => $v) {
    if (in_array($k, $props)) {
        $obj->$k = $v;
    }
}

return $obj;
}

Of course, you can put this in your DataObject class if you can contrive the class name of the generated class. In which case you replace the 2 static::class instances, with a string of the correct sub-class making it so you don't have to change the auto-generated classes that exist or the process generating them.

[Derokorian]

sneakyimp;11055387 wrote:

Ok having some luck.

I had forgotten that the magic method __set is only called when a property is inaccessible:

that said, this appears to bounce non-existent property assignments pretty well:

 	public function __set($prop, $value) {
		throw new Exception_Data_Object($prop . " is not a valid property for " . static::class);
 	}

Yeah, but it doesn't prevent setting public properties which are anywhere within the class hierarchy. (since they will be accessible in all scopes)

[sneakyimp]

Derokorian;11055389 wrote:

Maybe you need to rethink how these properties are defined tho (for example, above you have a public static that will not change, sounds like that should be a const?).

If I'm not mistaken, defining class constants as arrays requires only the most recent versions of PHP (5.6 & 7 I think) which is not feasible for this project.

Derokorian;11055389 wrote:

A single class should only be responsible for one thing.

I agree that we disagree on this point then 🙂

You show me a class with a method and I'll tell you at least two things that it does 😉
My point is that, while this might be a helpful as a broad heuristic guideline, the concept of "just doing one thing" is open for debate.

Derokorian;11055389 wrote:

Well assuming you can update the auto-generation of the classes, you can make the constructor private in dataobject, and then make a static constructor as part of the generated class. Something like:

public static function create($vars)
{
    $ref = new ReflectionClass(static::class);
    $props = array_map(function ($prop) {
        // returns just the property name
        $prop->name;
    }, array_filter($ref->getProperties(ReflectionProperty::IS_PUBLIC), function ($prop) {
        // removes static properties and anything further up the hierarchy
        return !$prop->isStatic() && $prop->class === static::class;
    });

$obj = new static();
foreach ($vars as $k => $v) {
    if (in_array($k, $props)) {
        $obj->$k = $v;
    }
}

return $obj;
}

That's some nice code-fu -- and you seem to have covered all the bases, but it doesn't look especially readable for the peons. I tried modifying my auto-generated DB classes to include a static array var which contains the db column names.

Derokorian;11055389 wrote:

Of course, you can put this in your DataObject class if you can contrive the class name of the generated class. In which case you replace the 2 static::class instances, with a string of the correct sub-class making it so you don't have to change the auto-generated classes that exist or the process generating them.

You've understood my problem well and I appreciate your insight a lot! It was contriving/deriving this class name that was worrying me. I would be forced to either rely on my class naming convention (ew! yuck!) OR maybe define some additional const in the DB class themselves to distinguish them from parent and child classes in the inheritance hierarchy.

[sneakyimp]

Derokorian;11055391 wrote:

Yeah, but it doesn't prevent setting public properties which are anywhere within the class hierarchy. (since they will be accessible in all scopes)

Yes that's the terrible downside of making properties public without getters/setters. So far it seems OK to have my db-column-properties public -- the point is to make it easy to work with the db, not harder. In the event I encounter problems, I can hopefully alter my auto-generating script to make db-column-properties protected and then flesh out the set and get functions to control things.