Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How can I reduce duplication in constants?

I have this Perl script with many defined constants of configuration files. For example:

use constant  {
LOG_DIR                             => "/var/log/",
LOG_FILENAME                        => "/var/log/file1.log",
LOG4PERL_CONF_FILE                  => "/etc/app1/log4perl.conf",
CONF_FILE1                          => "/etc/app1/config1.xml",
CONF_FILE2                          => "/etc/app1/config2.xml",
CONF_FILE3                          => "/etc/app1/config3.xml",
CONF_FILE4                          => "/etc/app1/config4.xml",
CONF_FILE5                          => "/etc/app1/config5.xml",
};

I want to reduce duplication of "/etc/app1" and "/var/log" , but using variables does not work. Also using previously defined constants does not work in the same "use constant block". For example:

use constant {
LOG_DIR                             => "/var/log/",
FILE_FILENAME                       => LOG_DIR . "file1.log" 
};

does not work.

Using separate "use constant" blocks does workaround this problem, but that adds a lot of unneeded code.

What is the correct way to do this?

Thank you.

like image 779
Tom Feiner Avatar asked Oct 23 '08 08:10

Tom Feiner


2 Answers

Using separate "use constant" blocks does workaround this problem, but that adds a lot of unneeded code.

Does it really?

use constant BASE_PATH => "/etc/app1";

use constant  {
    LOG4PERL_CONF_FILE                  => BASE_PATH . "/log4perl.conf",
    CONF_FILE1                          => BASE_PATH . "/config1.xml",
    CONF_FILE2                          => BASE_PATH . "/config2.xml",
    CONF_FILE3                          => BASE_PATH . "/config3.xml",
    CONF_FILE4                          => BASE_PATH . "/config4.xml",
    CONF_FILE5                          => BASE_PATH . "/config5.xml",
};

I don't see a lot of problems with this. You have specified the base path in one point only, thereby respecting the DRY principle. If you assign BASE_PATH with an environment variable:

use constant BASE_PATH => $ENV{MY_BASE_PATH} || "/etc/app1";

... you then have a cheap way of reconfiguring your constant without having to edit your code. What's there to not like about this?

If you really want to cut down the repetitive "BASE_PATH . " concatenation, you could add a bit of machinery to install the constants yourself and factor that away:

use strict;
use warnings;

use constant BASE_PATH => $ENV{MY_PATH} || '/etc/apps';

BEGIN {
    my %conf = (
        FILE1 => "/config1.xml",
        FILE2 => "/config2.xml",
    );

    for my $constant (keys %conf) {
        no strict 'refs';
        *{__PACKAGE__ . "::CONF_$constant"}
            = sub () {BASE_PATH . "$conf{$constant}"};
    }
}

print "Config is ", CONF_FILE1, ".\n";

But at this point I think the balance has swung away from Correct to Nasty :) For a start, you can no longer grep for CONF_FILE1 and see where it is defined.

like image 129
dland Avatar answered Nov 15 '22 07:11

dland


I'd probably write it like this:

use Readonly;

Readonly my $LOG_DIR            => "/var/log";
Readonly my $LOG_FILENAME       => "$LOG_DIR/file1.log";
Readonly my $ETC                => '/etc/app1';
Readonly my $LOG4PERL_CONF_FILE => "$ETC/log4perl.con";

# hash because we don't have an index '0'
Readonly my %CONF_FILES => map { $_ => "$ETC/config$_.xml" } 1 .. 5;

However, that's still a lot of code, but it does remove the duplication and that's a win.

Why are your logfiles numeric? If they start with 0, an array is a better choice than a hash. If they're named, they're more descriptive.

like image 41
Ovid Avatar answered Nov 15 '22 08:11

Ovid