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:: = \%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 [EMAIL PROTECTED] -----Original Message----- From: Robert May [mailto:[EMAIL PROTECTED] Sent: Tuesday, April 18, 2006 4:24 PM To: Brian Millham Cc: perl-win32-gui-users@lists.sourceforge.net 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... > > 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. --- 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
