in reply to code for review
For example, if you modify your "read_outpath()" subroutine to handle the "file not found" condition, you'll need only one copy of your "default value" for $data, instead of four copies (and the code will be a lot easier for human readers to understand):
That's one of many cases where a some more thought about the task and the conditions can lead the elimination of repeated code, which in turn leads to easier maintenance and legibility.sub read_outpath { if ( open FILE, '<', $outpath ) { read FILE, $data, 100000000; close FILE; return 1; } else { $data = <<EOT; <html> <head><title>ISSUE REPORT<\/title><\/head> <body bgcolor="#CCFFCC"><h1 align="center">MISMATCH REPORT</h1> <table width="1200px" border="10" align="center" frame="border" rules +="all"> <thead><tr bgcolor="#FFCCFF"> <th>ARTICLE ID</th><th>FILE MISSING IN PATH</th> <th>S100 OUTPUT</th><th>F300 OUTPUT</th> <th>DIFFERENCES</th></tr></thead> <tbody></tbody> </table> </body> </html> EOT } return 0; }
One other very important problem here: lack of documentation, especially about what the command-line args are supposed to be. You're logic implies some very strange expectations about what goes into @ARGV, and you really should provide at least a SYNOPSIS of the usage. It would be even better if you provide a DESCRIPTION of what the program is supposed to do, maybe even explaining the various conditions that it handles.
If you can describe the program's logic (i.e. document it) in a clear, concise way, it's likely you'll find it easier to write clear, concise code to implement that logic, getting rid of a lot of useless and redundant chunks of ugly stuff.
|
|---|