in reply to Convert BMP to HTML

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 סתם עוד האקר של פרל,

Replies are listed 'Best First'.
Re^2: Convert BMP to HTML
by harangzsolt33 (Deacon) on Nov 02, 2022 at 20:37 UTC
    Thank you for the suggestions! The super long sub is actually an experiment. I wrote a short one, and I noticed that there are a lot of variables, and passing them around to sub routines is no fun. For example, the part that decodes custom format BMPs would require at least 14 arguments. So, I've tried to break up this sub into 5-6 smaller sub-routines, and the result is not nice. The code didn't look pretty. So, I don't know what to do...

    "An excellent rule of thumb is that each subroutine should be fully visible on the screen without scrolling."

    Yeah, I know... It's wonderful when you have a small easy task to do, and you can do it in 35 lines of code and then return with the result. I like those kind of subs too. Then you have these monster subs that look like never-ending code and even when you try to divide the code into sections, it looks like a plate of spaghetti. Variables are created in one sub, then they are used in other subs all over. It can be a nightmare.

    The only reason I like to keep my scripts compatible with Perl 5.004 is because that way they work on the earliest Perl interpreter I have in my possession. If a Perl script is backward compatible, it will still run on newer versions of Perl! It just means that it will also run on older versions too. And having a script that can run on anything is great. It's one less thing to worry about.

    I didn't realize that putting the main program in brackets keeps the scope of variables out of the rest of the program. Actually, I remember reading about it. But I forgot that I can do that.

    "undef $HEADER;"

    I have played around a lot with string variables and I noticed that if I have a very large string in a sub, exiting the sub will not destroy the big string in memory! That's unusual, but we have talked about this before in this thread: Memory usage double expected and doing undef $VARIABLE causes Perl to free up that memory.

    But let me say a word about BMP image format as well. Whoever invented this stuff was really not thinking about how hard it's going to be for programmers who just want to decode a simple image. BMP images are usually stored upside down in the file BUT NOT ALWAYS. When the highest bit of the ImageHeight is 1, that means the BMP image is actually stored right side up. But it also means that you have to negate the ImageHeight before you can use it. BMP images also contain padding. And lots of it. It's all over the place. The header itself has a few bytes of empty space. The palette usually has a bunch of empty space in it (about 256 bytes or so). And each line of the image has padding at the end. The padding depends on how wide the image. In RLE compressed BMP files, the end of each line has an extra null byte SOMETIMES but not always. The BMP file header contains several different variables to describe the image format. One is used for compression. Okay, this value is stored in a 32-bit unsigned int, but it only has 4 VALID VALUES: 0, 1, 2, 3. Zero means no compression is used at all. One and two means RLE compression is used. And 3 indicates that it's a custom-format BMP file (which means that the red, green and blue values do not appear where they normally do in standard uncompressed BMP file. Normally, you have BLUE, then GREEN, then RED in this order. But in a custom-format BMP file, the RGB values can be in any order, and they can take up 1-8 bits or even more. There is total flexibility here.) So, the compression is stored in 32 bits but only the first two bits are used. We store the BITS_PER_PIXEL (BPP) in 16 bits but only the first 6 bits are ever used. Then there are the COLORS value which is 32 bits but never uses more than 8 bits. The FILESIZE and DATASIZE, on the other hand, uses ONLY 32 bits to store the obvious, and they are completely unnecessary. In fact, most programs ignore these values, because they CAN BE ZERO sometimes. The BMP header has a variable size. And it also has version number. There are specific versions, and each one is slightly different. There's version 12, 16, 40, 54, 56, 108 and 124. After version 40, which is considered the "standard," there was little change. But the fact that you have so many unused space in a file and so many rules makes it only harder to decode the image. The designers of BMP weren't thinking about SIMPLICITY. Even a 5-year-old could design a better file format.

    Consider the SUN Raster image file format, for example. It has a 32-byte header which is ALWAYS 32 bytes long and always holds exactly eight 32-bit unsigned integers in big-endian format. They contain the image width and height, the palette size, datasize, compression and a few other things. VERY SIMPLE. You don't need a 300-line sub to decode it. Even the RLE compression algorithm they use is A LOT nicer and more efficient than what BMP uses. RAS files do not use padding and don't waste space, and usually a simple uncompressed image is going to be somewhat smaller than a BMP file. Not only it's smaller, but the decoder program is a lot simpler too. I really wish RAS images were more popular than BMP, but today RAS is hardly even known. It's literally a non-existent format nowadays.

[reply]

      Hi harangzsolt33,

      The only reason I like to keep my scripts compatible with Perl 5.004 is because that way they work on the earliest Perl interpreter I have in my possession. ... And having a script that can run on anything is great. It's one less thing to worry about.

      Okay, but that just begs the question: why do you want to support legacy versions of the perl interpreter? Upgrading to shiny new1 versions gives you at least three benefits:

      • Improved Perl syntax, as mentioned previously.
      • Enhanced performance.
      • Access to a much wider range of CPAN modules.

      With a few minor exceptions,2 your old scripts should still run fine on the more modern perl interpreters.

      I have played around a lot with string variables and I noticed that if I have a very large string in a sub, exiting the sub will not destroy the big string in memory! ... and doing undef $VARIABLE causes Perl to free up that memory.

      Are you sure you were using lexical variables when you observed this?

      The super long sub is actually an experiment. I wrote a short one, and I noticed that there are a lot of variables, and passing them around to sub routines is no fun. ... So, I've tried to break up this sub into 5-6 smaller sub-routines, and the result is not nice. The code didn't look pretty. So, I don't know what to do...

      That’s a pretty good indication that an object oriented approach is called for.

      1Well, new-ish. The latest poll made me realise that my version of Strawberry Perl (the newest they offer) is now two years out of date. :-(
      2For example, on newer versions (5.26 and later) the current directory . is no longer included in @INC by default. This is actually a Good Thing.

      Cheers,

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

[reply]
[d/l]
[select]
[reply]

In Section Cool Uses for Perl