DStaal has asked for the wisdom of the Perl Monks concerning the following question:

I've got a program, that's a combination of procedural and object-oriented code. (Basically: It reads through a file, picking objects out as it goes, to verify structure.) It's all in one file, to make distribution/use easier. (And because it's small enough.)

Here's a couple of sections: (All under use strict; use warnings;, of course.)

use constant ERROR => 1;
use constant VERBOSE_ERROR => -1;
use constant WARNING => 2;
use constant VERBOSE_WARNING => -2;

# Handles all output of errors/warnings, etc.
sub output ($$) {
    my ($message, $level) = @_;
    if ( $error_flag && $level == ERROR ) {
        print "ERROR: $message\n";
    }
    if ( $warning_flag && $level == WARNING ) {
        print "WARNING: $message\n";
    }
        return;
}

[download]

And, later in the same file:

{
package State::Object;

use Scalar::Util qw(refaddr);

my %dses_name;
my %first_name;
my %last_name;
my %mid_intial;
my %internal_email;
my %full_name;
my %dn;
my %display_name;
my %line_address;

sub DESTROY {
    my ($self) = @_;
    
    no warnings qw(uninitialized);
    if (     defined($first_name{$$self}) or defined($last_name{$$self
+})
            or defined($mid_intial{$$self}) or defined($full_name{$$se
+lf})
            or defined($display_name{$$self}) ) {
        &main::output("Record for: '".$dn{$$self}."' starting at line:
+ ".$line_address{$$self}.' never ended.', 2);
    }
    
    delete $dses_name{$$self};
    delete $dn{$$self};
    delete $line_address{$$self};
}

# Other object code
...
}

[download]

Which works just fine, so far. (It's checking to see if we got an 'end of object' for each object here. All output is funnelled through the one function, with the intent of allowing multiple output formats at some point in the future.) I just don't like line 23 of the second section: It works, but I was hoping for some expert way to avoid using magic numbers,extended package specifications, or re-defining constants all over the place.

So, my question is: What's the best magic to use here? (And yes: I know that the 'output' function is non-optimal and incomplete, in lots of ways. At the moment it's basically a placeholder to make sure everything else works, then I'll go back an make it work well.)

Replies are listed 'Best First'.
Re: Mixing procedural and OO code, one file.
by ikegami (Patriarch) on Feb 05, 2009 at 16:26 UTC

    Some quick comments.

    You placed a prototype ("($$)") on output (bad) and you use "&" on the call (bad) which means "ignore the prototype". That makes no sense (bad).

    Why don't you place output in a module instead of main. You could even import it if you so desired. Actually, it's already been done.

[reply]
[d/l]
[select]

      I'm ignoring the prototype in this usage because Perl liked it a little better this way. (I put it on while debugging, as I was finding all my typing errors.) Most of my usuages don't ignore it. (I'm still undecided if I like prototypes for this type of case or not.) You do have a point: I can drop the '&' now that I've found all the other typos that were causing me trouble.

      I could put it in an module instead of main: But I'd still need it in main and in this scope. I do not want to require external modules (besides the standard) if at all possible: This is for wide distribution within the organization, and I want to have as little barriers as possible to it getting used. (We'll have enough trouble as it is: I expect some of the departments don't even have Perl on their machines...) It needs to look like a simple script to anyone installing and running it.

      Are you saying the easiest way to do this is to create an 'output' package and import it into both main and State::Object?

[reply]
[d/l]
[select]

        This is for wide distribution within the organization, and I want to have as little barriers as possible to it getting used.

        The installer for yours module can install the other module at the same time. That's a simple matter of putting one line in Makefile.PL.

        Are you saying the easiest way to do this is to create an 'output' package and import it into both main and State::Object?

        Seems simple to me: 

        use Logger qw( output :err_levels );

        [download]
        use strict;
use warnings;

package Logger;

use Exporter qw( import );

our @EXPORT_OK = qw(
   LOG_ERROR
   LOG_VERBOSE_ERROR
   LOG_WARNING
   LOG_VERBOSE_WARNING
   output
);

our %EXPORT_TAGS = (
   err_levels => [qw(
      LOG_ERROR
      LOG_VERBOSE_ERROR
      LOG_WARNING
      LOG_VERBOSE_WARNING
   )],
);

use constant {
   LOG_ERROR   => 1,
   LOG_WARNING => 2,
};

use constant {
   LOG_VERBOSE_ERROR   => -LOG_ERROR,
   LOG_VERBOSE_WARNING => -LOG_WARNING,
};

sub output {
   ...
}

1;

        [download]

        I find it odd that control over whether the warning or error is verbose or not is in the caller's control and not a configuration item.

        Update: Added code.

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom