Thank you for the suggestions Rob!
I'm working on incorporating the suggestions that you made.
Once I'm to the point of creating the Makefile and PPM, I'll probably =
need
some help.
I'll also have to do some reading up on testing...
BTW, when I made the change to allow the module to be true =
Win32::GUI::Skin,
I had to add:
*Skin:: =3D \%Win32::GUI::Skin::;
to make it work. (I figured that out from looking through GUI.pm) Why?
About documentation. I've created separate POD documents. I've noticed
that GUI.pm doesn't contain the POD. It this something that happens =
during
install (POD is extracted and removed from the original code), or just =
the
way the GUI.pm was done?
I realize that it's a basic question, but this is my first attempt at a =
real
module.
Brian Millham
This message traveled at least 44,000 miles to reach you!
Creator of the DW6000 Monitor
http://www.millham.net/dw6000
bmillham@...
-----Original Message-----
From: Robert May [mailto:rmay@...
Sent: Tuesday, April 18, 2006 4:24 PM
To: Brian Millham
Cc: perl-win32-gui-users@...
Subject: Re: [win32-gui] RE: [perl-win32-gui-users] The skin module
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...
>=20
> 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=20
warnings are reported from the user code perspective (if it's a user=20
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=20
cause a closure over $self, and does a GetDC that gets a DC that is not=20
released until the program finishes. You may have intended this for=20
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=20
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=20
automatically inherited from 'Universal' - you just need the=20
$Skin::VERSION variable set.. Change
my $VERSION =3D eval("0.1");
to
our $VERSION =3D 0.10;
2 decimal paces on $VERSION is more CPAN friendly. Additionally, you=20
don't need the eval, unless you want to use the CPAN 'alpha' notation=20
with versions like 0.10_01, in which case do:
our $VERSION =3D "0.10_01"; # for MakeMaker
$VERSION =3D eval $VERSION; # for Perl
(See perldoc perlmodstyle. perlnewmod, perlmod, perlmodlib are also=20
good reading)
(5) You'll need a Makefile.PL eventually (I can help here if you need=20
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=20
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 -=20
Test::Pod::Coverage ignores such functions and doesn't insist you write=20
documentation for it.
Regards,
Rob.
---
avast! Antivirus: Inbound message clean.
Virus Database (VPS): 0616-2, 04/18/2006
Tested on: 4/18/2006 5:05:29 PM
avast! is copyright (c) 2000-2003 ALWIL Software.
http://www.avast.com
---
avast! Antivirus: Outbound message clean.
Virus Database (VPS): 0616-2, 04/18/2006
Tested on: 4/18/2006 9:55:59 PM
avast! is copyright (c) 2000-2003 ALWIL Software.
http://www.avast.com
|