Re: Detecting scoping/namespace conflicts

[stephen]

in reply to Detecting scoping/namespace conflicts

I'll start with a brief discussion of my and local. However, the best way of dealing with this situation is to pull out the $localdata assignment into a subroutine. See the end of this note for a discussion of that.

Good: use 'my' not 'local'

Use my not local. local is used primarily for making a local-scope copy of a global variable; for example:

#!/usr/bin/perl

use strict;

sub foo {
   local $, = '... ';
   print ('a', 'list', 'of', "things\n");
}

foo();

print "This", "is", "a", "test\n";

prints out:

a... list... of... things
Thisisatest

my, on the other hand, always creates a new lexically-scoped variable that is only visible in the enclosing block or file. Use my for all of the variables that you yourself create. So, to modify your code:

use strict;

# Transform 'local' to 'my', and get rid of needless
# initialization to 'undef'
my $usefuldata = 1;  # in real life, lots of logic here
  
  if ( blah_blah() ) {
   # Eliminated second definition of $usefuldata
   $usefuldata = 2; # in real life, lots of logic here
  }
  insert_into_database($usefuldata)

Note: code untested

Better: Refactor To Subroutine

Problem is, $usefuldata serves no purpose here except as a placeholder. Nobody is incrementing it; nobody is altering it. It's either 1 or 2. (I know this is a simplified case, but the point still holds no matter how complex the logic.) It's far better from your code's standpoint to pull out the code that calculates $usefuldata into its own subroutine.

use strict;

# Put all of the $usefuldata stuff in a subroutine
sub get_useful_data
{

  # Eliminated initial assignment of $usefuldata...
  # There's no need to calculate it if blah_blah()
  # is true and we're going to replace the value anyway

  if ( blah_blah() ) {
    # No need to store $usefuldata in a variable...
    # just return it
    return 2;
  }
  else {
    # Since blah_blah() is false, we return the default
    # case
    return 1;
  }
}

# Now we can calculate $usefuldata exactly where we want it
insert_into_database( get_useful_data() )

Note: Code untested

An advantage to this is that you've eliminated $usefuldata entirely, and never need to wonder if somehow some other bit of code might have altered it. Everything having to do with $usefuldata is in one place.

Gets off soapbox

Update: Looking back, I'm not quite sure if you intended '1' to be the default case, or if the other $usefuldata was completely unrelated. This note assumes that you intended '1' to be inserted if your 'if' statement was false. In any case, replacing the variable assignment with a subroutine will solve the problem, because redefining a subroutine will cause a warning under '-w' anyway.

stephen