Clearing the decks: removing ancient Perlisms and stripping down

The next task is getting App::WebWebXNG to build, let alone pass any tests.

First up: I’ve changed the name of the page archive library, so I need to change the use statement, and fix up the new() call (making it direct invocation syntax while I’m at it).

The defined %hash syntax is no longer valid, so we need to fix that. The usages we have in this script are really “is there anything in this hash” checks, so keys will work to fix these.

it uses a lot of globals. This results from repackaging a Perl 4 script and making as few changes as possible to get it running. The vast majority are defined in webwebx.pl, but there are a couple – no, sorry, a bunch – that come from the CGI script. We need to add a use vars for these. Found two on the first run, then after the defined %hash issues were fixed, there were a bunch more. Adding them as we go.

“Replacement list is longer than search list”. There’s an interesting one! This is a tr that should be an s//g.

Okay,. load test passes! It doesn’t actually do anything, but that much is working. Good.

Let’s go look at the CGI script and see what it’s doing to initialize the globals we had to add; those are going to have to be set up somehow (for now, I think I’ll just add a setup_kludge function to do it). The variables we’re setting up here are mostly related to knowing where the script is hosted so that the internal link URLs are right, the location of the static files, and the location that stores all the data. Mojolicious should allow us to dispense with a lot of this and build the URLs as relative rather than absolute.

Now for some serious cleaning up. Let’s set up Perl::Tidy and Perl::Critic. Perl::Tidy is pretty critical, because the indentation is all over the place, and it’s hard to read the code. And Perl::Critic is just good insurance. I’m using policies similar to those we used at Zip.

Running those found a lot of things that needed neatening up…and several outright bugs!

  1. App::WebWebXNG had one perlcritic issue, a my with a trailing conditional. Not too bad for 25-year-old code.
  2. However, PageArchive::RCS had a lot of things to fix up.
    1. No use warnings. Okay, that one’s pretty easy.
    2. Tried to set the Rewound attribute for a directory; the code was after a return so it couldn’t be reached. When it was moved to be reachable, it was using a variable that didn’t exist! Needed to be using the instance variable for the object.
    3.  All of the open() calls used the old two-argument syntax. It’s still supported but it’s lousy practice, so I edited all of the open() calls in App::WebWebXNG and in PageArchive::RCS.
    4. There were several places where an if(my $foo... referenced $foo outside of the block. This changed sometime between Perl 5.6 and 5.38 (which I’m testing this with), so all of those had to be moved outside of the block.
    5. Finally, one method in PageArchive::RCS tried to use $self without creating it in scope. This would result in never getting error messages back, and may have hidden other bugs. We’ll see.

We’re back to all tests passing, perlcritic happy, and perltidy happy.  Created the repo on GitHub, pushed the work to date. Hang on, need to add a WIP marker…okay, got it.

A good morning’s work!

Reply