Re: [Maypole-dev] Re: Patch to support compound primary keys

From: Simon Flack (sf at flacks.net)
Date: Mon Dec 06 2004 - 10:59:53 GMT


Marcus Ramberg wrote:
> Simon Flack wrote:
>
>> On Mon, 6 Dec 2004 09:59:28 +0100, Marcus Ramberg wrote
>>
>>
>>> On 6. des. 2004, at 01.29, Simon Flack wrote:
>>>
>>>
>>>
>>>> On Mon, 6 Dec 2004 00:39:26 +0100, Marcus Ramberg wrote
>>>>
>>>>
>>>>> populate $r->objects from $r->{args}[0]
>>>>>
>>>>> For example, in my case, the the wiki I'm building, I could use this
>>>>> to set this to
>>>>> sub Mitiki::Page::populate_objects {
>>>>> my ($class,$r) = @_;
>>>>> my @obj = Mitiki::Page->search( node => $r->{args}->[0] );
>>>>> $r->objects([$obj[0]]);
>>>>> }
>>>>>
>>>>> (since I use a numeric id, not the node name, for pk but that's what
>>>>> I'll want to reference)
>>>>>
>>>>> Did that clear it up?
>>>>>
>>>>
>>>> Yes, but that's more or less what retrieve() is for:
>>>>
>>>> =head2 retrieve
>>>>
>>>> This turns an ID into an object of the appropriate class.
>>>>
>>>> Adding something so similar with a different name is a bit
>>>> confusing. Isn't
>>>> this just an example of a case where you need to define your own
>>>> Model::process
>>>>
>>>
>>> Note that even tho retrieve exists in the maypole *documentation*,
>>> it's in fact not implemented by maypole at all, but by class::dbi,
>>> which documents it as follows:
>>>
>>
>>
>> I don't dispute that. But the purpose of Model::retrieve() in Maypole is
>> already defined and it's purpose is more or less what your requesting
>> from
>> populate_objects.
>>
>>
> Yes, and I assume that the idea behind documenting it is so that you can
> override it in subclasses of ::Base, however, for Class::DBI based
> classes this is impossible, which is why we need a better name. Also,
> passing $r to such a method instead of args would make imore sense since
> allows for more flexibility.
>
>> I don't know that. I thought that Simon had written an application
>> with an
>> Email::Store model.
>>
>>
> yes... well... Email::Store *IS* based on class::dbi.. The model in use
> is Maypole::Model::CDBI::Plain. I have the start of a
> DBIx::SearchBuilder model lying around, but apart from that, not much
> exists as far as I know.
>
>> Here's another possibility. Define your own get_object_id() to
>> translate the
>> name to an id:
>>
>> sub Mitiki::Page::get_object_id {
>> my ($class,$r) = @_;
>> my @obj = Mitiki::Page->search( node => $r->{args}->[0] );
>> $r->objects([$obj[0]]);
>> return $obj[0]->id;
>> }
>> If I understand CDBI's memory cache correctly, because you store the
>> object
>> temporarily, the subsequent retrieve() in process() shouldn't result in
>> another database hit.
>>
>> But yes, we could deprecate retrieve() and replace it with
>> populate_objects()
>> or similar. I'm just worrying about creating a sprawling API. Also, if
>> you
>> accept that M::M::CDBI isn't going to cater directly for every
>> concievable
>> CDBI model, then it makes sense to optimise it for the common cases.
>> And I'm
>> not sure whether this is a common case.
>>
>>
> Your proposed solution feels like a hack. The fact is, ::Base is too
> CDBI centric, and this would be a good way to start remedying that..
> populate_objects would be an abstract method in ::Base, and implemented
> with retrieve in ::CDBI ... That means that CDBI is optimised for the
> common cases (retrieving by PK from args) and easily overridable for
> custom actions ( not populating at all, populating from node, whatever
> you like). And the best part is, this won't break anything in existing
> apps.

Ok, lets go with this. But I think I'd rather it be fetch_objects, i.e.
it shouldn't set objects itself, just return them. I think that'll make
it more reusable.

--simonflk

_______________________________________________
maypole-dev mailing list
maypole-dev at lists.netthink.co.uk
http://lists.netthink.co.uk/listinfo/maypole-dev



This archive was generated by hypermail 2.1.3 : Thu Feb 24 2005 - 22:25:57 GMT