Login | Register
My pages Projects Community openCollabNet

Discussions > dev > Re: [propel-dev] roadmap

propel
Discussion topic

Hide all messages in topic

All messages in topic

Re: [propel-dev] roadmap

Reply

Author =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Full name =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Date 2006-03-28 16:07:17 PST
Message > Good questions -- and that's why this is a discussion rather than a
> decision. I personally do think that if it is added at all it
> should be
> toggle-able, yes. API docs is a good argument for keeping more
> traditional methods. Bear in mind, though, that the getter/setter
> methods are all doing almost the exact same thing, so I think from a
> readability perspective consolidating that would help. Also, if I
> understand the proposals, you could still always override the
> methods in
> the stub methods, so for the important stuff the API docs would still
> work as expected.

true

> -- I haven't looked over these proposals yet, but I guess in my mind I
> see __call() as a replacement for the _standard_ get*()/set*()
> methods &
> not for the other methods of the class. Others may have other ideas.

agreed

Re: [propel-dev] roadmap

Reply

Author =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Full name =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Date 2006-03-28 16:03:39 PST
Message > I have a few things that need to be considered before using __call/
> __get/__set
>
> 1) For those of us that want API documentation for our codebase, I
> don't think we can get it unless the functions are actually there.

I don't think it's _that_ much of a deal. The function name alone
tells you which field it gets or sets, and the already generated
documentation is absolutely useless. You usually only document
extensively (for instance, optional parameters, behavioral stuff etc)
if you implement your own code in the stubs, and that's where it's
still possible.

> 2) How do __call/__get/__set work with subclasses? How does
> overriding work?

It works as usual, no change there.

> 3) If this behavior is added, shouldn't it be TOGGLE-ABLE?

Yeah why not we can have a thousand different builders if people
desire ;)

> 4) Why do you want to do this? Before we go do this level of
> changing, where are the performance results that show these changes
> would make more than a negligible improvement? For instance, most
> of the getters are one-liners; and most of the setters are 3-liners.

For a table with many fields, these add up quickly, especially in
case of getters and setters for related objects. I expect the
difference in parse time to be quite significant.

Re: [propel-dev] roadmap

Reply

Author Cameron Brunner <cameron dot brunner at gmail dot com>
Full name Cameron Brunner <cameron dot brunner at gmail dot com>
Date 2006-03-28 15:47:22 PST
Message On 3/29/06, Oliver Schonrock <oliver.schonrock​@realtsp.com> wrote:
> > Alan Pinstein wrote:
> >> 4) Why do you want to do this? Before we go do this level of changing,
> >> where are the performance results that show these changes would make
> >> more than a negligible improvement? For instance, most of the getters
> >> are one-liners; and most of the setters are 3-liners.
>
> I am with Alan on this one. __call is a nasty hack, which doesn't
> document well and doesn't really solve anything (certainly not speed).

The documentation issue as far as i see it is mostly a moot point, its
frustrating but can easily enough be worked around, we can make a 2nd
tree that phpdoc can parse if its really that much of an issue, the
question is a lot about how it performs. We have a rather large code
bloat in propel generated files currently that is obvious on a site
with a lot of tables and references which is, imo, 1 of the main
things needed to be addressed in propel 2.

Before anyone says that call/get/set/watevr does/doesnt help speed (im
not sure weather it would either way myself, it still has to run just
as much code, if not more, less parse time on the classes tho and you
could load the extra join classes on request not before), i would
prefer to see some benchmarks of how they would react on a 'big' site.


Cameron

Re: [propel-dev] roadmap

Reply

Author Oliver Schonrock <oliver dot schonrock at realtsp dot com>
Full name Oliver Schonrock <oliver dot schonrock at realtsp dot com>
Date 2006-03-28 15:09:45 PST
Message > Alan Pinstein wrote:
>> 4) Why do you want to do this? Before we go do this level of changing,
>> where are the performance results that show these changes would make
>> more than a negligible improvement? For instance, most of the getters
>> are one-liners; and most of the setters are 3-liners.

I am with Alan on this one. __call is a nasty hack, which doesn't
document well and doesn't really solve anything (certainly not speed).

Now if we can stop propel classes having to extend BaseObject and
implement Persistant, then we have something closer to

POPOs (that's short for plain old PHP objects) ;-)

The biggest reason that Torque didn't make it is that it violated the
basic KISS principle. If propel is to succeed long term is should aim to
"extend" as little as possible and be as true to the language as practical.

Oliver

Re: [propel-dev] roadmap

Reply

Author hlellelid
Full name Hans Lellelid
Date 2006-03-28 12:43:57 PST
Message Hi Alan,

Good questions -- and that's why this is a discussion rather than a
decision. I personally do think that if it is added at all it should be
toggle-able, yes. API docs is a good argument for keeping more
traditional methods. Bear in mind, though, that the getter/setter
methods are all doing almost the exact same thing, so I think from a
readability perspective consolidating that would help. Also, if I
understand the proposals, you could still always override the methods in
the stub methods, so for the important stuff the API docs would still
work as expected.

-- I haven't looked over these proposals yet, but I guess in my mind I
see __call() as a replacement for the _standard_ get*()/set*() methods &
not for the other methods of the class. Others may have other ideas.

Hans

Alan Pinstein wrote:
> I have a few things that need to be considered before using
> __call/__get/__set
>
> 1) For those of us that want API documentation for our codebase, I don't
> think we can get it unless the functions are actually there.
> 2) How do __call/__get/__set work with subclasses? How does overriding
> work?
> 3) If this behavior is added, shouldn't it be TOGGLE-ABLE?
> 4) Why do you want to do this? Before we go do this level of changing,
> where are the performance results that show these changes would make
> more than a negligible improvement? For instance, most of the getters
> are one-liners; and most of the setters are 3-liners.
>
> I am not sure that the overhead required by __call etc would be LESS
> than the direct function call itself... don't forget that call has to
> check to see if the function exists anyway, then you have to have a
> bunch of code in the __call routine to "do the right thing".
>
> So let's see some evidence that a suggested change will actually improve
> performance before making a change that causes the code to be less
> readable, less documented, and of unknown performance benefit!
>
> 5) Maybe there is some level of abstraction that could be used to reduce
> the code?
>
> For instance, the dts-style functions have a lot of code that could
> probably be factored out into the base class to reduce these generated
> functions from 20 lines of code to a single line. Same is true of the
> related object fetching, setters, etc.
>
> Alan
>
>
> On Mar 28, 2006, at 10:28 AM, Hans Lellelid wrote:
>
>>> * Utilizing dynamic __call/__get/__set in BaseObject/BasePeer classes
>>> and kill much generated code from the Object/Peer classes for
>>> get*/set*/add*
>>>
>>> If noone dislikes this I'll add two dev discussion pages to the wiki.
>>>
>>
>> Adding the pages sounds good to me. The only concern I have (which I
>> can mention in comments on wiki too) is that certain methods (temporal
>> methods) take additional parameters -- which might present a problem
>> __call/__get/__set, unless we just standardize on an API that does not
>> support these. Just something to discuss.
>
>
> --------------------​--------------------​--------------------​---------
> To unsubscribe, e-mail: dev-unsubscribe@prop​el.tigris.org
> For additional commands, e-mail: dev-help at propel dot tigris dot org
>

Re: [propel-dev] roadmap

Reply

Author Alan Pinstein <apinstein at mac dot com>
Full name Alan Pinstein <apinstein at mac dot com>
Date 2006-03-28 12:29:31 PST
Message I have a few things that need to be considered before using __call/
__get/__set

1) For those of us that want API documentation for our codebase, I
don't think we can get it unless the functions are actually there.
2) How do __call/__get/__set work with subclasses? How does
overriding work?
3) If this behavior is added, shouldn't it be TOGGLE-ABLE?
4) Why do you want to do this? Before we go do this level of
changing, where are the performance results that show these changes
would make more than a negligible improvement? For instance, most of
the getters are one-liners; and most of the setters are 3-liners.

I am not sure that the overhead required by __call etc would be LESS
than the direct function call itself... don't forget that call has to
check to see if the function exists anyway, then you have to have a
bunch of code in the __call routine to "do the right thing".

So let's see some evidence that a suggested change will actually
improve performance before making a change that causes the code to be
less readable, less documented, and of unknown performance benefit!

5) Maybe there is some level of abstraction that could be used to
reduce the code?

For instance, the dts-style functions have a lot of code that could
probably be factored out into the base class to reduce these
generated functions from 20 lines of code to a single line. Same is
true of the related object fetching, setters, etc.

Alan


On Mar 28, 2006, at 10:28 AM, Hans Lellelid wrote:

>> * Utilizing dynamic __call/__get/__set in BaseObject/BasePeer classes
>> and kill much generated code from the Object/Peer classes for
>> get*/set*/add*
>>
>> If noone dislikes this I'll add two dev discussion pages to the wiki.
>>
>
> Adding the pages sounds good to me. The only concern I have (which I
> can mention in comments on wiki too) is that certain methods (temporal
> methods) take additional parameters -- which might present a problem
> __call/__get/__set, unless we just standardize on an API that does not
> support these. Just something to discuss.

Re: [propel-dev] roadmap

Reply

Author hlellelid
Full name Hans Lellelid
Date 2006-03-28 09:39:47 PST
Message David Zülke wrote:
> Am 28.03.2006 um 19:24 schrieb Hans Lellelid:
>> Yeah, the problem doesn't really apply to __call(), you're right. More
>> for __get()/__set():
>>
>> Current API:
>>
>> print "Date is " . $obj->getDateValue("Y-m-d");
>>
>> Desired API (as I understand it):
>>
>> print "Date is " . date("Y-m-d", $obj->dateValue);
>
> I don't think we should use accessors. Let's stick to getDateValue().
> We'll have one __call() method that handles getters and setters for all
> fields. Makes the generated classes a lot slimmer ;) Also, we actually
> _have_ to do it this way because you couldn't implement custom stuff in
> the stubs otherwise.
>

That's a good point. I guess overriding __get / __set would be
extremely cumbersome. Using __call() would definitely help keep things
slim, though, true.

Hans

Re: [propel-dev] roadmap

Reply

Author =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Full name =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Date 2006-03-28 09:35:18 PST
Message Am 28.03.2006 um 19:24 schrieb Hans Lellelid:
> Yeah, the problem doesn't really apply to __call(), you're right.
> More
> for __get()/__set():
>
> Current API:
>
> print "Date is " . $obj->getDateValue("Y-m-d");
>
> Desired API (as I understand it):
>
> print "Date is " . date("Y-m-d", $obj->dateValue);

I don't think we should use accessors. Let's stick to getDateValue().
We'll have one __call() method that handles getters and setters for
all fields. Makes the generated classes a lot slimmer ;) Also, we
actually _have_ to do it this way because you couldn't implement
custom stuff in the stubs otherwise.

- David

Re: [propel-dev] roadmap

Reply

Author hlellelid
Full name Hans Lellelid
Date 2006-03-28 09:25:10 PST
Message David Zülke wrote:
>> Adding the pages sounds good to me. The only concern I have (which I
>> can mention in comments on wiki too) is that certain methods (temporal
>> methods) take additional parameters -- which might present a problem
>> __call/__get/__set, unless we just standardize on an API that does not
>> support these. Just something to discuss.
>
> No problem IMHO, call_user_func_array() is our friend.
>
>> We may also want to have the "lightweight" OM classes be an option.
>> It'd be nice to have several different options for types of classes to
>> generate, but I also recognize that these would have to be maintained :)
>
> _Having_ them would be easy, but you're right, maintaining them would be
> the real issue ;)
>
> I added 4, 5 and 6 to the "known priorities". Or shall I move them to
> the second list. What exactly is the difference anyway? ;)

Yeah, I don't know :) I think the first list is stuff we've discussed &
sort of resolved to add, whereas the second list is stuff that has been
requested & that might need a bit more discussion before we actually sit
down & write task tickets.

(I guess that means that we can sit down & write task tickets for the
first part of the list.)

Hans

Re: [propel-dev] roadmap

Reply

Author hlellelid
Full name Hans Lellelid
Date 2006-03-28 09:24:14 PST
Message David Zülke wrote:
>>> Adding the pages sounds good to me. The only concern I have (which I
>>> can mention in comments on wiki too) is that certain methods (temporal
>>> methods) take additional parameters -- which might present a problem
>>> __call/__get/__set, unless we just standardize on an API that does not
>>> support these. Just something to discuss.
>>
>> No problem IMHO, call_user_func_array() is our friend.
>
> Wait a minute, you were talking about the other way round, right? No
> problem either, __call gets an array of params anyway...!?

Yeah, the problem doesn't really apply to __call(), you're right. More
for __get()/__set():

Current API:

print "Date is " . $obj->getDateValue("Y-m-d");

Desired API (as I understand it):

print "Date is " . date("Y-m-d", $obj->dateValue);

... so basically we wouldn't be able to support the date formatting
parameter. Probably not a big deal, but a difference from current system.

Hans

Re: [propel-dev] roadmap

Reply

Author =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Full name =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Date 2006-03-28 08:18:18 PST
Message >> Adding the pages sounds good to me. The only concern I have (which I
>> can mention in comments on wiki too) is that certain methods
>> (temporal
>> methods) take additional parameters -- which might present a problem
>> __call/__get/__set, unless we just standardize on an API that does
>> not
>> support these. Just something to discuss.
>
> No problem IMHO, call_user_func_array() is our friend.

Wait a minute, you were talking about the other way round, right? No
problem either, __call gets an array of params anyway...!?

- David

Re: [propel-dev] roadmap

Reply

Author =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Full name =?ISO-8859-1?Q?David_Z=FClke?= <dz at bitxtender dot com>
Date 2006-03-28 08:15:03 PST
Message > Adding the pages sounds good to me. The only concern I have (which I
> can mention in comments on wiki too) is that certain methods (temporal
> methods) take additional parameters -- which might present a problem
> __call/__get/__set, unless we just standardize on an API that does not
> support these. Just something to discuss.

No problem IMHO, call_user_func_array() is our friend.

> We may also want to have the "lightweight" OM classes be an option.
> It'd be nice to have several different options for types of classes to
> generate, but I also recognize that these would have to be
> maintained :)

_Having_ them would be easy, but you're right, maintaining them would
be the real issue ;)

I added 4, 5 and 6 to the "known priorities". Or shall I move them to
the second list. What exactly is the difference anyway? ;)

- David

Re: [propel-dev] roadmap

Reply

Author hlellelid
Full name Hans Lellelid
Date 2006-03-28 07:28:05 PST
Message Soenke Ruempler wrote:
> Hi Hans,
>
> Hans Lellelid <mailto:hans at velum dot net> wrote on Tuesday, March 28, 2006
> 4:50 PM:
>
>> I put together a Roadmap wiki page:
>>
>> http://propel.phpdb.​org/trac/wiki/Develo​pment/Roadmap
>>
>> I incorporated some things David & I have discussed in the
>> past, but at
>> this point it's more of a discussion than a finalized plan, so please
>> feel free to use the comment feature to post suggestions of features
>> you'd like to see (and hopefully help implement) in future
>> development.
>>
>> I also added a Criteria page (linked from Roadmap) to discuss some of
>> the things that need attention in current Criteria API. We
>> can add new
>> pages to think through issues like identifier quoting & identity
>> map/"cache" stuff too. Oscar & Cameron, feel free to add
>> pages -- e.g.
>> Development/IdentifierQuoting, Development/IdentityMap -- to
>> outline how
>> these systems could be implemented; it might be helpful to have some
>> single points of reference that aggregates & addresses our what-if
>> concerns with these solutions.
>
> I'd like to add two points:
>
> * Utilizing dynamic __call/__get/__set in BaseObject/BasePeer classes
> and kill much generated code from the Object/Peer classes for
> get*/set*/add*
> * Making the copy() method more flexibel (limiting the level of
> deepcopy, adding table-excludes)
>
> If noone dislikes this I'll add two dev discussion pages to the wiki.
>

Adding the pages sounds good to me. The only concern I have (which I
can mention in comments on wiki too) is that certain methods (temporal
methods) take additional parameters -- which might present a problem
__call/__get/__set, unless we just standardize on an API that does not
support these. Just something to discuss.

We may also want to have the "lightweight" OM classes be an option.
It'd be nice to have several different options for types of classes to
generate, but I also recognize that these would have to be maintained :)

Hans

RE: [propel-dev] roadmap

Reply

Author Soenke Ruempler <ruempler at topconcepts dot com>
Full name Soenke Ruempler <ruempler at topconcepts dot com>
Date 2006-03-28 07:22:41 PST
Message Hi Hans,

Hans Lellelid <mailto:hans at velum dot net> wrote on Tuesday, March 28, 2006
4:50 PM:

> I put together a Roadmap wiki page:
>
> http://propel.phpdb.​org/trac/wiki/Develo​pment/Roadmap
>
> I incorporated some things David & I have discussed in the
> past, but at
> this point it's more of a discussion than a finalized plan, so please
> feel free to use the comment feature to post suggestions of features
> you'd like to see (and hopefully help implement) in future
> development.
>
> I also added a Criteria page (linked from Roadmap) to discuss some of
> the things that need attention in current Criteria API. We
> can add new
> pages to think through issues like identifier quoting & identity
> map/"cache" stuff too. Oscar & Cameron, feel free to add
> pages -- e.g.
> Development/IdentifierQuoting, Development/IdentityMap -- to
> outline how
> these systems could be implemented; it might be helpful to have some
> single points of reference that aggregates & addresses our what-if
> concerns with these solutions.

I'd like to add two points:

* Utilizing dynamic __call/__get/__set in BaseObject/BasePeer classes
and kill much generated code from the Object/Peer classes for
get*/set*/add*
* Making the copy() method more flexibel (limiting the level of
deepcopy, adding table-excludes)

If noone dislikes this I'll add two dev discussion pages to the wiki.

--

soenke
Messages per page: