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

Ignoramus10782 <ignoramus10782@xxxxxxxxxxxxxxxxxxxx> wrote:
On 11 Apr 2006 10:56:41 -0700, Paul Lalli <mritty@xxxxxxxxx> wrote:
Ignoramus10782 wrote:

Why aren't you using warnings?

No good reason, I guess. I added 'use warnings' to the module. It did
not change anything,


It did not change anything *yet*.

warnings can be triggered due to data, so it may be that you just
haven't yet hit any data that generates a warning.


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

Hm, I am open minded, but why should I do that? What is the reason?


"Coping with Scoping":

http://perl.plover.com/FAQs/Namespaces.html

See footnote 3:

Added 2000-01-05: Perl 5.6.0 introduced a new our(...) declaration.
Its syntax is the same as for my(), and it is a replacement for
use vars...


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

I use them to cache data to speed up access. The data is static and I
do not want it to be computed every time.


And you think that lexical variables can't be used for that?

They can.

A good rule of thumb is:

You should prefer lexical variables over package variables,
except when you can't.

And you can here.


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?

I agree. I changed that line.


You agree here, and disagree with the same suggestion farther down...


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': $!";

I used the $!


Good idea.


and decided to skip the lexical suggestion.


Why did you decide to skip the lexical suggestion?

Every college freshman knows that globals are "bad", yet you choose
to use globals when a properly scoped alternative is available.

If you tell us the reason why you think that a lexical filehandle
won't work for you, then we might be able to suggest a way to
work around whatever that objection might be.


while ( $_ = <CONFIG> ) {

chomp;

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

Everything is fine now and these problems no longer occur. However, I
agree that I should print the $/ should the problem ever reoccur, so I
changed my script top print $/.


Huh?

Rather than insert code that would properly deal with the problem
should it reoccur, you choose to fail with a message instead?

It would be way better to simply change your code as has been suggested:

{ # create a scope for $/
local $/ = "\n";
while ( $_ = <CONFIG> ) {
chomp;
... # don't call functions that muck with $/ here!
}
}


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.
^^^^^^^^^^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^^^^^^^^^^
Instead of carp, I simply print the filename.


You seem to have completely missed the point here, while you have
already agreed to this point above.

Knowing that there is a problem in Algebra/Config.pm is not
helpful enough if there are 200 programs that use Algebra::Config.

Knowing where the function _call_ is would be more helpful, since
that is probably where the bad bad bad input string is being
introduced.


--
Tad McClellan SGML consulting
tadmc@xxxxxxxxxxxxxx Perl programming
Fort Worth, Texas
.

Relevant Pages