Re: [CDBI] Different subclasses for one table / Factory Pattern?

Stephan Brunner wrote:
> I wrote:
>>> You have probably come up with a solution by now, I'm sure.  Let us
>>> know what you ultimately came up with.
>> Hmm, well, yes, somehow... But I didn't have the time to implement my
>> solution, and, in my experience, the solution might change during
>> implementation;-)
>>
>> For the next weeks, I'm away from the computer, so please be patient. But I
>> will let you know and I'm also interested in the discussion!
> 
> Here I am back again.
> Looking at Oliver's episodes, I'm quite happy about the solution I came up 
> with: I kept everything related to different-subclasses-per-table away from 
> CDBI.
> 
> I introduced a class FileType with several subclasses (FileType::JPG, 
> FileType::AVI etc.). To the class representing the database table ("files"), 
> I added an object property "type", wich keeps an object of the relevant 
> subclass of FileType. Looks like so:
> 
> package File;
> use base 'Class::DBI';
> # ...
> sub type {
>     my $self = shift;
>     if (not exists $self->{type}) {
>         $self->{type} = FileType->new($self);
>     }
>     return $self->{type};
> }
> 
> package FileType;
> sub new { 
>     shift; # throw class away
>     my $file = shift;
>     my $name = $file->name();
> 
>     my $class = undef;
>     my @classes = qw/JPG WAV AVI/;
> TRY: foreach my $try_class (@classes) {
>         $try_class = __PACKAGE__ . '::' . $try_class;
>         # passing $name explicitly avoids the call of $file->name in
>         # every is_member:
>         if ($try_class->is_member($name, $file)) {
>             $class = $try_class;
>             # first try_class saying "yes, $file is_member!" gets selected
>             last TRY;
>         }
>     }
>     if (not defined $class) {
>         $class = __PACKAGE__ . '::Unknown';
>     }
> 
>     # create and return an object of the selected class;
>     # keep a reference to the $file object for later use
>     my $self = bless { file => $file }, $class;
>     return $self;
> }
> # ...
> 
> package FileType::JPG;
> # for example
> sub is_member {
>     shift;
>     my ($name, $object) = @_;
>     return ($name =~ /.jpe?g$/i) ? 1 : 0;
> }
> 
> 
> One big drawback is the additional method calls I get. To rotate a jpg, for 
> example, I call $file->type->rotate(), as rotate is a method of 
> FileType::JPG.
> And, even worse, in FileType::JPG::rotate (called on the FileType object 
> stored in $file->type), I call $self->file() to get the file that actually 
> will be rotated. To avoid this, I would have to call 
> $file->type->rotate($file), which also looks strange to me.
> 
> Well, this obviously is no perfect OO design. On the other hand, it is quite 
> clear and obvious in how it works as I'm not using nor overriding any CDBI 
> internals.
> Anyway, I'm not perfectly happy about it, so any comments still very welcome!
> 
> Regards,
> Stephan

Calling $self->file() in a method doesn't seem at all bad, so long as
you're storing a reference to your file and not having to look it
up/construct it again.

Rather then defining a new method 'type' which makes the assumption that
CDBI objects are hashrefs (which that are, and is unlikely to change,
but still...), would it not be better to just use a has_a relationship
with your class?

package File;
use base 'Class::DBI'

__PACKAGE__->columns(Essential => qw/id data/);
__PACKAGE__->has_a(data => 'FileType',
	inflate => 'new');

Or something similar.

-Oli

_______________________________________________
ClassDBI mailing list
ClassDBI-Ra3b/[email protected]
http://lists.digitalcraftsmen.net/mailman/listinfo/classdbi

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)

iD8DBQFFCl6lSbWXajvTQ+URAsD5AJ4yDMaB0kAD74vYOeq5y2QUzwPS5wCfWlmi
gscx/62m0KnQmR1QRqrUdw8=
=W3x+
-----END PGP SIGNATURE-----