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

Am Freitag 15 September 2006 10:04 schrieb Oliver Jeeves:
> Stephan Brunner wrote:
[...]
> > 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};
> > }
[...]

> > 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?

Oops, yes, you're right! I obviously wasn't aware that I'm dealing with a 
class that is mostly designed not in my own code, but in CDBI.

> package File;
> use base 'Class::DBI'
>
> __PACKAGE__->columns(Essential => qw/id data/);
> __PACKAGE__->has_a(data => 'FileType',
> 	inflate => 'new');

Ok, but this would require an additional column in the database (which would 
be redundant)?

> Or something similar.

(In the meantime, some googling...)

No, it wouldn't -- there's the column group TEMP which can store the filetype 
data only on the object without touching the database. I will definitly try 
this to avoid the type()-method on package File. Thank you for the hint!

> -Oli

Stephan