Feedback on current O/R Mapping Unittests

[Ren]

Let me put this another way... what exactly is the purpose of having datatypes in the model map templates, if you have to manually map the datatypes anyways?

For validation.
Sqlite, for example, will quite happily store a large blob into an integer column.

[thr]

http://www.sitepoint.com/forums/show...48&postcount=1

Hmm, that's pretty much what I do with my "ObjectQuery" (in another way ofc. mapping joins and conditions with __set/__get).

Though I remembered that looking alot nicer.

Still a nice idea tho, gotta investigate it further

Just the methods is probably good enough, another possibility with 5.1 is use Reflection*::getDocComment() and use a marker keyword in that.

Ah yes, but I realy wanna get away from the "docBlock" comments... it feels so, well.. tacky ;p.

You've completely missed what I was saying.

Not at all, but trying to insert a string in a int field will generate an invalid query and rollback the whole transaction.

Let me put this another way... what exactly is the purpose of having datatypes in the model map templates, if you have to manually map the datatypes anyways?

... what Ren said pretty much.

It's not that I don't see your point dreamscape, I just don't agree with you.

[dreamscape]

Not at all, but trying to insert a string in a int field will generate an invalid query and rollback the whole transaction.

I think you did miss what I was saying, because what I suggested that would never happen.

[thr]

I think you did miss what I was saying, because what I suggested that would never happen.

No, I know about the auto-integer cast you suggested - but say that you for some reason get the string "hello" into age and it then casts it to int(think it becomes 1 or 0) - you get invalid data.

[thr]

The current api would allow this:

<?php
// This does not allow null on givenName/familyName and adds them to constraint group 1
$template->givenName    = Model::stringField(10, false, 1);
$template->familyName    = Model::stringField(20, false, 1);

// This allows null on email and age, and adds them to constraint group 2
$template->email        = Model::stringField(25, true, 2);
$template->age            = Model::integerField(3, true, 2);
?>

Which would result in this sql table:

create table `Person` (
	`id` int( 11 ) not null ,
	`firstName` varchar( 10 ) not null ,
	`givenName` varchar( 10 ) not null ,
	`email` varchar( 25 ) ,
	`age` smallint( 3 ) ,
	primary key ( `id` ) ,
	unique (
		`firstName` ,
		`givenName`
	),
	unique (
		`email` ,
		`age`
	),
) engine = InnoDB;

Edit: Ok, on second thought the code above would not be valid as you can't have constraints on NULL fields(email/age) but the example is still valid.

On another note, should the "$allownull" be false or true as standard?

[thr]

Ok, some more "status" updates - currently the ClassMaps handle inheritance flawlessly, take this example:


classes/Person.php:

<?php
class Person implements Persistable
{

    public $name;
    public $email;
    public $age;
    public $groups;
    
    protected $parent;
    protected $savings;
    
    public function setParent($val)         { $this->parent = $val; }
    public function getParent()                { return $this->parent; }
    public function setSavings($savings)    { $this->savings = (double)$savings; }
    public function getSavings()             { return $this->savings; }
    public function getNameEmailLink()         { return "<a href='mailto{$this->email}'>{$this->name}</a>"; }
    
}
?>

classes/Member.php:

<?php
class Member extends Person
{

    public $joindate;
    
}
?>

classes/Admin.php:

<?php
class Admin extends Member
{

}
?>

templates/Person.php:

<?php
$template = new ModelClassMap("Person");

$template->name        = Model::stringField(10, true);
$template->email    = Model::stringField(25, true);
$template->age        = Model::integerField(3);
$template->groups    = Model::collectionField("Group");

$template->setSavings(Model::doubleField(100, true));
$template->setParent(Model::objectField("Person"));
?>

templates/Member.php:

<?php
$template = new ModelClassMap("Member");
$template->joindate = Model::integerField(11);
?>

templates/Admin.php:

<?php
$template = new ModelClassMap("Admin");
$template->joindate = Model::integerField(11, true);
$template->email = Model::stringField(50, true); 
?>

Person is the superclass for all three, Member defines one aditional field named joindate (it also inherits all of Persons public properties and public methods ofc, and thier definitions in the classmap so no need to define them again) which is an Integer and can't be null.

Admin then extends Member, but there is not a required joindate for admins and thus it changes the "allownull" for joindate to true. The email adress is not required either for the admin so he changes the field define in his parents(Member) parent(Person) to allow null.

This example works flawlessly, tests.php:

<?php
require 'UnitTests/Helpers/dev_funcs.php';
require_all('./');
require_all('Interfaces/');
require_all('UnitTests/Classes/');

$cfg = new ModelConfiguration;
$cfg->database("mysql", "localhost", "development", "dev", "devnull");
$cfg->paths("UnitTests/Classes/", "UnitTests/Templates/", "UnitTests/Templates/Cache/");

$model = new Model($cfg);

$template = $model->loadClassMap("Person");
$person = new Person;
$person->name = null;
$person->email = "thrthr@gmail.com";
$person->age = 20;
$person->groups = array();
$person->setSavings(0.0);
$person->setParent(new Person);
$template->validateObject($person);

$template = $model->loadClassMap("Member");
$member = new Member;
$member->name = null;
$member->email = "thrthr@gmail.com";
$member->age = 20;
$member->groups = array();
$member->setSavings(0.0);
$member->setParent(new Person);
$member->joindate = 1;
$template->validateObject($member);

$template = $model->loadClassMap("Admin");
$admin = new Admin;
$admin->name = null;
$admin->age = 20;
$admin->groups = array();
$admin->setSavings(0.0);
$admin->setParent(new Person);
$admin->joindate = 1;
$template->validateObject($admin);
?>

Edit: On second thought, maybee you shouldn't be allowed to change a superclass set validation of a field... right?

[johno]

I've played around with lastcrafts idea of "magic" declarative definitions.

    class Node {
        protected $id;
        protected $sid;
        protected $parentId;
        protected $position;
        protected $visible;
        
        public static function getMapping() {
            $mapping = new ClassMapping('nodes');
            $mapping->id->primaryKey;
            $mapping->sid->string;
            $mapping->parentId('parent_id')->null->default(null);
            $mapping->visible->boolean->default(true);
            $mapping->setUniqueKey('parentId', 'sid');
            return $mapping;
        }
    }
    
    class Article extends Node {
        protected $publishedOn;
        protected $title;
        protected $body;
        
        protected $authors;
        
        public static function getMapping() {
            $mapping = new ExtendingClassMapping('articles');
            $mapping->title->string;
            $mapping->body->string;
            $mapping->publishedOn('published_on')->custom(new DateFieldMapping())->null->default(null);        
            return $mapping;
        }
    }
    
    
    class Authorship {
        protected $articleId;
        protected $authorId;
        
        public static function getMapping() {
            $mapping = new ClassMapping('authorships');
            $mapping->articleId('article_id');
            $mapping->authorId('author_id');
            $mapping->setPrimaryKey('articleId', 'authorId');
            return $mapping;
        }
    } 

Not implemented yet, but I really don't see a problem. So what do you think? You can easily use those primary key and unique key constraints for identity map. That's what I am trying to do now.

[thr]

I think it's way to messy and I prefer to have set Classes/Objects to handle field mapping, such as in my last post. I played around with lastcrafts idea also but It just get's a bit to when you say wan't to add a primary key,

$mapping->id->primarykey->not_null->auto_increment->integer;

that's just like writing Sql ;p

[johno]

I think it's way to messy and I prefer to have set Classes/Objects to handle field mapping, such as in my last post. I played around with lastcrafts idea also but It just get's a bit to when you say wan't to add a primary key,

$mapping->id->primarykey->not_null->auto_increment->integer;

that's just like writing Sql ;p

I think you missunderstood me. I don't want you to write DB table definitions like this. Its messy. All I was trying to say is to rewrite this.

$template = new ModelClassMap("Person");

$template->name        = Model::stringField(10, true);
$template->email    = Model::stringField(25, true);
$template->age        = Model::integerField(3);
$template->groups    = Model::collectionField("Group"); 

Into something like this.

$template = new ModelClassMap("Person");
$template->name->string(10)->null;
$template->email->string(25)->null;
$template->age->integer(3);
$template->groups->collection("Group"); 

Yes, its just syntax sugar.

[thr]

I think you missunderstood me. I don't want you to write DB table definitions like this. Its messy. All I was trying to say is to rewrite this.

Nope, didn't missunderstand you altho my example was a bit stupid I agree ;P. Just don't like that approach, feels strange to say the least.

[thr]

Ok, new idea:

<?php
class TestModel extends Model
{

    protected function PersonMap()
    {
        $person = $this->createMapping("Person");
        $person->name->string(25)->notNull();
        $person->age->integer(11)->notNull();
        $person->parent->object("Person");
        $person->groups->collection("Group");
    }
    
    protected function GroupMap()
    {
        $group = $this->createMapping("Group");
        $group->name->string(25)->notNull();
        $group->comment->string(255);
        $group->members->collection("Person");
    }

}
?>

Yes I used the magic __set/__get here ;p

Idea is this: You have your basic "abstract class Model" which is the top level in the model heriarchy - to use objects with a model you extend it and define methods in your class named "ClassMap();" (as you can see above) which should be protected - this would allow you to builda heirarchy of model classes for example Module-based applications, etc. To hide a type of mapping from a subclass just make the method private instead.

This would also mean you have all your mappings in one single place.

[akrabat]

This would also mean you have all your mappings in one single place.

I like the code example, but I'm on the fence with having all the mappings in one place.

I think in terms of modules for my sites. A module consists of the table definition, the php code and the html templates. Thus, I'd quite like to keep everything related to the module together so it's easy to take to another site.

[Ren]

Think I'd split it.

   interface ModelMapContainer
   {
      function getMap(Model $model, $name);
   }

   class SimpleModelMapContainer implements ModelMapContainer
   {
      private $parent;
      
      function __construct(ModelMapContainer $parent = null)
      {
         $this->parent = $parent;
      }
         
      function getMap(Model $model, $name)
      {
         $methodName = $name.'Map';
         if (method_exists($this, $methodName))
            return $this->$methodName();
         if ($this->parent instanceof ModelMapContainer)
            return $this->parent->getMap($model, $name);
         return NULL;
      }
      
      function PersonMap()
      {
         ...
      }
   }
   
   class Model
   {
      protected $maps;
      
      function __construct(ModelMapContainer $mapContainer)
      {
         $this->mapContainer = $mapContainer;
      }
      
      function getMap($name)
      {
         if (!isset($this->maps[$name]))
            $this->maps[$className] = $this->mapContainer->getMap($this, $className);
         return $this->maps[$name];
      }
   } 

I think its more flexible, could still have an ModelMapContainer that can auto-sense the map from the database schema information, or from an XML file etc etc.

[thr]

Nice idea Ren, I agree that's a much better approach. (this is the exact reason I don't code everything and then release it as "this is done", perfer to debate / argue everything with others before making a decision)

      function __construct(ModelMapContainer $parent = null)
      {
         $this->parent = $parent;
      } 

This I'm interested in, does the above argument for the constructor(ModelMapContainer $parent = null) work for you, for me it just breaks the whole application without errors or anything.

[Dr Livingston]

Sorry for going off topic Thr, but you shouldn't post script that uses bad practices, and yes I know I'm just being picky

public function getNameEmailLink()         { return "<a href='mailto{$this->email}'>{$this->name}</a>"; }
    
} 

You could change to this instead - with a Visitor for example,

public function getNameEmailLink( PersonPrinter $printer )         { 
return $printer -> formatEmail( $this -> email, $this -> name );
}
} 

Another example, of what I have...

// ...
public function toHtml() {
return new PersonFormat( $this -> export() );
}
// ...

class PersonFormat {
// ...
public function getName() {
return $this -> lastname.' '.$this -> firstname;
}
// ...
}

$formatter = $person -> toHtml();
$name = $formatter -> getName(); 

Anyways, I'm having trouble following this thread now due to a lack of time, but you need to be congratulated for tackling this, well done and please continue...

[thr]

haha, ah well sorry for that ;p I just threw in a method that had get as prefix to demonstrate that it only takes valid set/get pairs.

[Ren]

Nice idea Ren, I agree that's a much better approach. (this is the exact reason I don't code everything and then release it as "this is done", perfer to debate / argue everything with others before making a decision)

This I'm interested in, does the above argument for the constructor(ModelMapContainer $parent = null) work for you, for me it just breaks the whole application without errors or anything.

Yes, I use it often.

class B
{
}

class A 
{
    private $b;

    function __construct(B $b = null)
    {
        $this->b = $b;
    }    
}
$a = new A();
var_dump($a); 

Outputs...

object(A)#1 (1) {
  ["b:private"]=>
  NULL
}

What version of PHP do you have? 5.1.2 here. Can't remember when it was added, thought it was awhile ago tho.

[thr]

got 5.1.2 also, gotta try again then ;P

[thr]

back to topic, anyways - I realy don't like using __set/__get magic if it can be avoided so here's my current version of what Ren wrote above:

<?php
interface MMapContainer{
    public function getMap($name);
}
class MClassMethodMap implements MMapContainer{
    
    function getMap($method){
        $method    = (string)$method . 'Map';
        if(method_exists($this, $method)){
            return $this->$method();
        }else{
            return null;
        }
    }
    
    function PersonMap(){
        $map = new Person;
        $map->name        = "string:25";
        $map->age        = "integer:3";
        $map->text        = "text:long";
        $map->groups    = array(new Group);
        $map->children    = array(new Person);
        $map->parent    = new Person;
    }
    
}
?>

Instead of using a "magic" class called ClassMapping you specify what you wan't by either asigning something of the correct type (ie array of groups for a group collection, etc.) or assign a string such as "string:25" or "integer:3", the default is also "NOT NULL" now, so you add :null at the end to allow null.

Good, bad, ugly?

[Dr Livingston]

Umm...

I have 5.1.1 at the moment, but I only ask a question though,

function __construct(B $b = null)
    {
        $this->b = $b;
    } 

Wouldn't PHP5 through an error if $b wasn't typeof B first, before the class method ever was processed by PHP no?

I just don't see the point or any benifit in giving a class method arguement a default, particularly when there is the use of type hints for enforcement; Or am I missing something?

[thr]

I just don't see the point or any benifit in giving a class method arguement a default, particularly when there is the use of type hints for enforcement; Or am I missing something?

Well, take the example above with Rens code, a Map doens't have to have a parent and thus you might want it to be null.

[Dr Livingston]

Yes, but NULL isn't the type expected no? Say... I must be thick this morning

[thr]

Yes, but NULL isn't the type expected no? Say... I must be thick this morning

Well in pretty much all programinglanguages that use objects you can have null instead of an object.

[Ren]

Hmm few methods on Model? Seems to make sense.

getType($type, $allowsNull = true, $length = null, $precision = null)
getHasOneType('Person' .... );
getHasManyType('Groups', .... );
getValueType('', ...columns... );

$person->name = $model->getType('string', false, 10);
$person->age = $model->getType('integer', false); 

[thr]

yeah you're probably right in that case, the approach I had above but with statics, it'll probably be best that way.