Brian Millham wrote:
Hi again,
I see the quite a few people checked out my page about creating a skinfile
for use with my Skin module. I realize that the documentation needs a lot
of work...
Has anyone tried to play with this yet?
I've just run it up. Win98, AS Perl 5.8.7 (build 813). Looks pretty!
Have some of you who are more familiar with creating modules looked at the
code in Skin.pm. Am I on the right track here for creating a true
Win32::GUI::Skin module?
Looks good so far ... some comments from a very quick skin of the code,
and in no particular order:
(1) Use the Carp module, rather than warn/die in your PM file, so that
warnings are reported from the user code perspective (if it's a user
problem, of course. If it's a module problem then warn/die are fine)
(2) It looks like your SetEvent('Paint', ...) call in the constructor
cause a closure over $self, and does a GetDC that gets a DC that is not
released until the program finishes. You may have intended this for
speed, but if so it would be better to create a new class with the calss
style CS_OWNDC, rather that hogging a shared resource.
(3) When storing your own instance data in the module I'd start the hash
variable names with something other than '-' (I tend to use '_') to
avoid any possible future clash with Win32::GUI instance data. See also
my (to be written) response to Jeremy about instance data.
(4) You don't need to provide a Version function - you'll get one
automatically inherited from 'Universal' - you just need the
$Skin::VERSION variable set.. Change
my $VERSION = eval("0.1");
to
our $VERSION = 0.10;
2 decimal paces on $VERSION is more CPAN friendly. Additionally, you
don't need the eval, unless you want to use the CPAN 'alpha' notation
with versions like 0.10_01, in which case do:
our $VERSION = "0.10_01"; # for MakeMaker
$VERSION = eval $VERSION; # for Perl
(See perldoc perlmodstyle. perlnewmod, perlmod, perlmodlib are also
good reading)
(5) You'll need a Makefile.PL eventually (I can help here if you need
it) so that the standard
perl Makefile.PL
make
make test
make install
incantation works. This also makes it easy to generate a PPM for easy
distribution to ActiveState perl users.
(6) Tests? At least test that the module loads with no errors - I guess
that testing much else will be hard.
(7) Documentation - but I see you already chastised yourself on this one.
(8) name class private methods starting with an underscore -
Test::Pod::Coverage ignores such functions and doesn't insist you write
documentation for it.
Regards,
Rob.
