http://www.perlmonks.org?node_id=639517

cosmicperl has asked for the wisdom of the Perl Monks concerning the following question:

Hi All,
  My new module is now available on CPAN. Before I go much further to implement more features I'd like to ask the all knowing (you guys) what you think, and whether there are any serious changes I need to make to the way I'm doing things. Also suggestions as to what features you'd like to see next would be good.

Please bare in mind this is only version 0.05, I really just wanted to get something up at this point.

I've named the module Image::Mate
http://search.cpan.org/~cosmicnet/

At the moment it allows you to create or load an image, change it's colour, draw lines, and save it. Sorry really not much at all. Next I'll be adding text capabilities, and some sort of distortion (putting together a more flexible captcha module than the ones I've found) and possible an adaptation of GD::Barcode (might need to create these from the current piece of software I'm working on).

So if you'll all lend me a bit of your advice that would be great :)

Lyle Hopkins
P.S. Before someone mentions the problems with the POD, I've already fixed it and it'll be looking right in the next version uploaded.

Replies are listed 'Best First'.
Re: Comments and suggestions on new module
by LTjake (Prior) on Sep 18, 2007 at 14:09 UTC

    I've noticed in almost every method that you had "if it's GD, do A, if it's Imager, do B, etc.." This screams to me that you need to do some deeper encapsulation.

    I would propose that you have Image::Mate::(GD|Imager|Image::Magick) and when new() is called bless your object to the user's preference.

    Just my 2 cents.

    --
    "Go up to the next female stranger you see and tell her that her "body is a wonderland."
    My hypothesis is that she’ll be too busy laughing at you to even bother slapping you.    " (src)

[reply]
      Ok... Care to elaborate with a quick code example?
[reply]
        package X;

our $PREF;

sub new {
    bless {}, $PREF;
}

package A;

sub foo {
    print __PACKAGE__;
}

package B;

sub foo {
    print __PACKAGE__;
}

package main;

$X::PREF = 'B';

my $o = X->new;

$o->foo;

        [download]

        Prints:

        B

        [download]

        You can change $X::PREF = line to 'A' and it will print A.

        --
        "Go up to the next female stranger you see and tell her that her "body is a wonderland."
        My hypothesis is that she’ll be too busy laughing at you to even bother slapping you.        " (src)

[reply]
[d/l]
[select]
Re: Comments and suggestions on new module
by tonyc (Pilgrim) on Sep 19, 2007 at 01:04 UTC

    As LTjake suggests, you might want to create a subclass for each of the imaging libraries you support.

    For example, you have a line() method, instead of defining that in Image::Mate with a huge conditional for each implementation, you simply define it in each of the derived classes. Then have Image::Mate::new() return an object of that type.

    Imager does something similar for its font support - Imager::Font itself doesn't know how to draw text, it lets Imager::Font::(Truetype|Win32|FreeType2|Type1) specialize _draw() which does all the heavy lifting.

    This is pretty basic OO.

    Another issue I see is that you appear to load every one of the imaging libraries even though the user would only want to use one of them, and you then call import() in package main for that module.

    The use of the c option to new() seems a bit inconsitent too - for GD this is used to decide between a paletted image and a true color image, for Imager it's used to decide between an 8-bits/sample image (24-bit color) and 16-bits/sample (48-bit color), but I suspect this is a case of trying to make sense of the different options the modules support.

[reply]
      Your right it is my trying to make sense of the different options they support. Not all the doc's are straight forward, certainly not for ImageMagick.

      I'll brush up some more on my OO, be be honest I haven't made any OO code with Perl before so it's all pretty new to me.
[reply]

Back to Seekers of Perl Wisdom