Hi harangzsolt33,

Thanks for this! As others have said, it’s cool. Here’s some feedback on the Perl code itself. I hope you find it constructive and useful.

# Tested with TinyPerl 5.8 on Windows XP and Perl 5.004 on DOS.

[download]

These are now very old. I see from your home page that Perl is a hobby for you, so you’re not constrained by workplace policies to use any particular version. I strongly recommend you upgrade to an up-to-date Perl; this will allow you to take advantage of newer syntax features. For example, instead of local *FILE you can use lexical filehandles (as of v5.6). And a statement like my $F = defined $_[0] ? $_[0] : ''; can be simplified to my $F = $_[0] // ''; using the defined-or operator // (as of v5.10).

my $REF = ReadBMP('D:\\DESKTOP\\mandel3.bmp', 3);
ReduceColorDistribution($REF);
my $HTML = Canvas2HTML($REF);
CreateFile('D:\\DESKTOP\\TESTING.HTM', $HTML);
exit;

[download]

Putting driver code at file scope can potentially lead to hard-to-find bugs. Consider:

use strict;

my $foo = 42;
print "$foo\n";     # 42. No surprises here
bar();
print "$foo\n";     # 17! How did that happen?

# ...hundreds of lines of code...

sub bar
{
    ...
    $foo = 17;      # Oops! Forgot "my"
    ...
}

[download]

As you can see, use strict only gets you so far. Since $foo is declared at file scope, it remains visible/accessible to all subroutines in the rest of the file. This action-at-a-distance is what use strict is designed to prevent; so, let strict do its job by enclosing top-level code in its own block:

MAIN:       # Label (not needed, just for documentation)
{
    my $REF = ReadBMP('D:\\DESKTOP\\mandel3.bmp', 3);
    ReduceColorDistribution($REF);
    my $HTML = Canvas2HTML($REF);
    CreateFile('D:\\DESKTOP\\TESTING.HTM', $HTML);
    exit;   # Not really needed either
}

[download]

Now $REF and $HTML are invisible/inaccessible to the rest of the file.

if ($ERR > 5) { undef $HEADER; return 0; }

[download]

The error handling looks a bit strange, but what caught my eye was the undef $HEADER; statement. This looks like a hangover from C programming, where a malloc must eventually be followed by a free to prevent a memory leak. But Perl is garbage collected; here, $HEADER is a lexical variable so it will be marked as eligible for collection as soon as it goes out of scope (viz., when the return is executed). The undef does no harm, but it also accomplishes nothing here.

And lastly,

sub ReadBMP
{
    ...
}

[download]

The body of this subroutine is 377 lines long! Well, we’ve all written the sub that wouldn’t end from time to time, but when this happens you really need to go back and refactor it into separate subroutines. An excellent rule of thumb is that each subroutine should be fully visible on the screen without scrolling. Otherwise, you’re creating problems for your future self. For example, this loop:

while ($H--)
{

[download]

caught my eye. Is the logic correct? (Because the operator is in postfix position, the condition is tested before the decrement is applied). On reflection, I think this is correct. But in order to check it, I wanted to see where $H was declared; and I had to go back 183 lines to find it! A maintenance nightmare.

Just my 2¢. Hope it helps,

Athanasius <°(((>< contra mundum סתם עוד האקר של פרל,

In reply to Re: Convert BMP to HTML by Athanasius
in thread Convert BMP to HTML by harangzsolt33

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.