Re: EXAMPLE -- Re: Strange issue with `CHOMP' not working...

Ignoramus10782 wrote:
I rewrote my module to make it slightly nicer.

Not sure if anyone is interested, but here it is:
==================================================

package Algebra::Config;

use strict;


Why aren't you using warnings?

require Exporter;

use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);

Are you specifically writing this module to be compatable with perl
5.005? May I ask why? If not, you should be enabling warnings and
using the 'our' keyword instead of the vars pragma


@ISA = qw(Exporter);
@EXPORT= qw( get_config_var get_data_dir get_algebra_root );
@EXPORT_OK = qw( get_config_var );

$VERSION = 2000.0426;

use vars qw( $algebra_data_dir $_values );

Why are these global variables? Are you planning on accessing them
outside of this module? They should probably be lexicals - especially
one that starts with an underscore, which traditionally refers to a
"private" variable.


sub get_algebra_root {
my $algebra_root = $ENV{'ALGEBRA_ROOT'} || $ENV{'DOCUMENT_ROOT'}
|| die "Webmaster has to define ALGEBRA_ROOT environment variable!";

Seems to me like this could be better as a 'croak' statement than a
die. Why do you care what line in get_algebra_root this happened?

return $algebra_root;
}

sub get_config_var {
my ($category, $variable) = @_;

return $_values->{$category}->{$variable} if exists $_values->{$category}->{$variable};

my $algebra_root = get_algebra_root;
my $config_file = "$algebra_root/etc/$category";

open( CONFIG, $config_file ) or die "Config file '$config_file' does not exist or is not readable.";

1) Use lexical variables instead of global barewords (again, unless
you're specifically developing for Perl 5.005 or earlier)
2) Use the three-argument form of open
3) Those are not the only two possible reasons the open could fail. Do
not guess - let Perl tell you specifically why, by including the $!
variable in the die() statement.

open my $CONFIG, '<', $config_file or die "Could not open
'$config_file': $!";

while ( $_ = <CONFIG> ) {

Why are you explicitly assigning to $_? Nothing "wrong" with this, I
guess, it just strikes me as odd. It would look more normal to assign
to a lexical variable, or to let the while(<>) magic happen
automatically.

chomp;

So for all your insistance that localizing $/ was the best suggestion
you got, you still completely ignored it? Why?


if ( /(\n|\r)/ ) {
my $str = substr( $_, 0, 80 );
print STDERR "Algebra::Config: BAD BAD BAD Input string after chomp but BEFORE regex: '$str'.\n";

'print STDERR' is generally better written 'warn', but in this case is
probably better written 'carp', so you know where the input came from.

}

s/(\n|\r)//g;

Seems silly to have to do this match twice. Why not simply make this
the condition of your test above?


next if( /^\s*#/ );
next if( /^\s*$/ );
my( $key, $value ) = split( /\s*=\s*/, $_ );

$_values->{$category}->{$key} = $value;
}

close( CONFIG );

return $_values->{$category}->{$variable} if exists $_values->{$category}->{$variable};

$_values->{$category}->{$variable} = undef;

return undef;
}


sub get_data_dir {
my ($category, $variable) = @_;

my $dir = get_config_var( $category, $variable );
if ( $dir !~ /^\// ) {

Yuck. I hate leaning toothpicks.

if ($dir !~ m{^/}) {

Of course, I also generally dislike regexps when a substr() will do,
so:

if (substr($dir, 0, 1) ne '/') {

$dir = $algebra_data_dir . "/$dir";

Why are you using both explicit concatenation and interpolation in the
same statement? Seems inconsistant.

$dir = "$algebra_data_dir/$dir";

}
return $dir;
}

$algebra_data_dir = get_config_var( "general", "algebra_data_dir" ) || $ENV{ALGEBRA_ROOT};

$_values = {};

1;

Paul Lalli

.

Relevant Pages

  • Re: Newbie questions, migrating from c++
    ... > to get no warnings using SEEK_SET etc, ... So if I'm to do something with chars in string (or perl ... It is not a good idea to use this method of subroutine invocation unless ... sub no_need { ...
    (comp.lang.perl.misc)
  • Re: Look At This Package
    ... I'm trying to translate my java objects to perl object. ... >> Execution of test.pl aborted due to compilation errors. ... > use warnings; ... > sub f_name { ...
    (perl.beginners)
  • Re: ignore directories with File::Find?
    ... file systems but still traverses hidden directories. ... no warnings 'File::Find'; ... sub find_directories { ... Perl isn't a toolbox, but a small machine shop where you ...
    (perl.beginners)
  • HASH vs Regexp
    ... use strict; ... use warnings; ... sub Regexp::matches { ... OTOH I wonder why in current perl references are not ...
    (comp.lang.perl.misc)
  • FAQ 7.13 Whats a closure?
    ... Closures are documented in perlref. ... closures are implemented in Perl as anonymous ... subroutines with lasting references to lexical variables outside their ... return sub; ...
    (comp.lang.perl.misc)