nathggns / Scaffold

Lightweight PHP API Framework
Other
8 stars 2 forks source link

Add a callback in a model for when a row is created / deleted / updated #106

Open nathggns opened 11 years ago

nathggns commented 11 years ago

The way I'd see it, is something like this.

<?php
class ModelUser extends ModelDatabase {
    public function init() {
        $this->on('create', function() {
            // ...
        });

        $this->on('delete', function() {
            // ...
        });

        $this->on('update', function() {
            // ...
        });
    }
}

The on method should be protected.

There will also be a trigger method to trigger an event.

nathggns commented 11 years ago

Maybe create this as a trait Events? Model could use it, and then it can be used elsewhere in the framework too. Applications can use it too. Similar to node's EventEmitter.

nathggns commented 11 years ago

Actually, ignore me… None of the methods should be protected…

ClaudioAlbertin commented 11 years ago

The question is whether this shouldn't be just a class. Shouldn't it be possible to create a bare Event object?

nathggns commented 11 years ago

What exactly would it do though? :S I'm pretty sure it should just be a trait. Why would you need an event object??  — Nathaniel Higgins http://nath.is

On Mon, Apr 22, 2013 at 5:54 PM, Claudio Albertin notifications@github.com wrote:

The question is whether this shouldn't be just a class. Shouldn't it be possible to create a bare Event object?

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/issues/106#issuecomment-16801248

nathggns commented 11 years ago

Needs tests.

ClaudioAlbertin commented 11 years ago

Indeed.

ClaudioAlbertin commented 11 years ago

It would enable using Events with the observator pattern or similar. It would just make it a lot more flexible to use. I doubt that a subclass A' of a class A needs events if A doesn't, so it would work with inheritance.

nathggns commented 11 years ago

Observer pattern? — Nathaniel Higgins http://nath.is

On Mon, Apr 29, 2013 at 11:44 AM, Claudio Albertin notifications@github.com wrote:

It would enable using Events with the observator pattern or similar. It would just make it a lot more flexible to use. I doubt that a subclass A' of a class A needs events if A doesn't, so it would work with inheritance.

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/pull/106#issuecomment-17159933

ClaudioAlbertin commented 11 years ago

Just about anything that keeps an object as a property instead of extending/using the class.

nathggns commented 11 years ago

Traits also work with inheritance too, though.  — Nathaniel Higgins http://nath.is

On Mon, Apr 29, 2013 at 2:05 PM, Claudio Albertin notifications@github.com wrote:

Just about anything that keeps an object as a property instead of extending/using the class.

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/pull/106#issuecomment-17165274

nathggns commented 11 years ago

Please make sure that all methods and variables are under_scored.

ClaudioAlbertin commented 11 years ago

I was under the seemingly wrong impression we were using lowerCamelCase for methods, like PSR-2 suggests.

nathggns commented 11 years ago

We really don't follow PSR yet. We'll look at following it as close as possible before we release V1 though. — Nathaniel Higgins http://nath.is

On Wed, May 1, 2013 at 7:03 PM, Claudio Albertin notifications@github.com wrote:

I was under the seemingly wrong impression we were using lowerCamelCase for methods, like PSR-2 suggests.

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/pull/106#issuecomment-17296215

ClaudioAlbertin commented 11 years ago

This is ready to be merged.

nathggns commented 11 years ago

I'll take a proper look at the code when I get home — Nathaniel Higgins http://nath.is

On Thu, May 2, 2013 at 5:07 PM, Claudio Albertin notifications@github.com wrote:

This is ready to be merged.

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/pull/106#issuecomment-17347393

nathggns commented 11 years ago

Ready to merge yet?

ClaudioAlbertin commented 11 years ago

No.

nathggns commented 11 years ago

What changes do you have planned?  — Nathaniel Higgins http://nath.is

On Sun, May 5, 2013 at 4:20 PM, Claudio Albertin notifications@github.com wrote:

No.

Reply to this email directly or view it on GitHub: https://github.com/Scaffold/Scaffold/pull/106#issuecomment-17453242

ClaudioAlbertin commented 11 years ago

Plenty. Technically you could merge this now, though. And I'll create a pull request with improvements at a later point.

nathggns commented 11 years ago

Nah, I'd rather wait for you to fix this. It isn't urgent.

nathggns commented 11 years ago

Also before this is merged, it needs to be implemented in ModelDatabase. I'm fine with doing that, so long as you implement your desired changes, @ClaudioAlbertin.

ClaudioAlbertin commented 11 years ago

Will be fixed on the 8th of June.