On Sat, Jun 13, 2009 at 00:54, David Christensen<dpchr...@holgerdanske.com> wrote: > Steve Bertrand wrote: >> If anyone has the time to review and report, I would much appreciate > it. >> http://ipv6canada.com/extra_billing.txt > > 1. I don't see a header comment block.
header comment blocks are wastes of space, that is what good source control and documentation is for. > > 2. I don't see comment section boundaries. What? > > 3. I don't see author information. Again, source control. > > 4. I don't see a copyright notice. If it is only for internal use this isn't so important. And at least in the US, all works get copyright protection regardless of whether you put a notice in or not. If this is meant to be distributed then there should be a whole bunch more files, one of which should explain the license and copyright information. > > 5. I don't see any POD. This is a good point. The real question is how much documentation it needs. It is not a module, so really only how to call it and what files it uses need to be documented in the POD. > > 6. I don't see command line options/ processing. I don't think it has command line options, all of its options come from a config file. > > 7. I don't see a usage message. A decent point, but since it looks like you are supposed to just run it a usage message would be sort of pointless. > > 8. I prefer 4 column indentation. I want a pony. If you want to start a flame war we can take this off list and yell and scream at each other about our respective religions, but it will probably not change either of our minds. But just to get things started: the only True Way is to use tabs for indent levels and spaces for alignment, this allows the user to set his or her indentation to whatever he or she wants by modifying how many spaces a tab displays. Example: if ($foo) { <tab>print "this is a foo\n", <tab> "there are many like it\n", <tab> "but this one is mine\n"; } > > 9. I prefer limiting my code to 72 columns. Here our religions agree, with the caveat of "where possible without making the code look worse", but it is still a religious issue, not a coding issue. > > 10. I don't see a version control system tag; I use CVS and put $Id$ > near the top of all my files. Version control is a key differentiator > between hacked code and professional systems software product. If your > company is using this script to bill people, your company and customers > expect the latter. RCS crap like $Id$ is a sign of no package management (i.e. just copying files to machines). Package management is a key differentiator between hacked together systems and professional systems. I should be able to ask the machine's package management system "what package does this file belong to and what version is that package" -- with a checksum comparison to ensure the file has not been tampered with. > > 11. Use h2xs to create a module and move your code into it. (Another > key differentiator.) Unless you are actually coding XS, you should use Module::Starter instead. > > 12. Do you have automated regression tests? (Another key > differentiator.) No disagreement there. > > 13. You're using a lot of 'my' variables to fetch fields from data > structures. I do this and/or suffer through with the full syntax, but > have always wanted a cleaner solution. Perhaps a full set of accessors? > AUTOLOAD? Alias? I just tend to use the data structure directly. I don't see much value in $obj->foo; over $obj->{foo}; unless this is a fully fledged OO system where you need to hide implementation details from the users of the class. -- Chas. Owens wonkden.net The most important skill a programmer can have is the ability to read. -- To unsubscribe, e-mail: beginners-unsubscr...@perl.org For additional commands, e-mail: beginners-h...@perl.org http://learn.perl.org/