1

Currently i am using following code to do find the string replace the string and print it to the out file with the same name of that in file but in the replaced folder

use Tie::File;
@files = <*>;
foreach $file (@files) {
    my $filename = $file;
    open(my $fh, '<:encoding(UTF-8)', $filename) or die "Could not open file '$filename' $!";
    open(NEWFILE,"> ./replaced/$filename");

   while(my $variable=<$fh>){

      s/Insertstoredprocedure ( / Insertstoredprocedure('$filename',/g;
      s/SuccessSp()/SuccessSp()('$filename')/g;

  print NEWFILE "$variable";
  print "done\n";
  }
}

this script is intended to replace all the thing and put the file into the replace folder with the changes.... this is not working it gives errror... how can i replace and print the same for all the files in the present directory ..

zdim
  • 64,580
  • 5
  • 52
  • 81
Abhishek
  • 101
  • 3
  • 8
  • 1
    Could you paste the error here? – Julio Aug 23 '18 at 07:57
  • @GerhardBarnard yes i am doing the same as u said – Abhishek Aug 23 '18 at 08:06
  • just wanted to replace for e.g name() with name('filename') – Abhishek Aug 23 '18 at 08:06
  • 3
    To my mind, if somebody has an error, the least he/she can do is put the error on the question. Even if the problem is obvious. – Julio Aug 23 '18 at 08:10
  • I finally edited the question and added the error by myslef. Please, for future problems always include the errors you get. – Julio Aug 23 '18 at 08:52
  • @Julio: Please don't do that. The error that you get from running the OP's code may be different from what they are experiencing for a number of reasons. Your edit has been rejected. – Borodin Aug 23 '18 at 10:59

2 Answers2

1

Direct errors and red flags:

  • Once you assign while ($variable = <$fh>) the $_ is not set to what <$fh> read; it is left as it has been (undefined here); so your regex, which matches it (by default), won't work

  • Parentheses to be matched as literal characters in regex need be escaped

  • The code processes all files from the current directory, <*> -- which in this code may well contain the script itself as well, and there are no guards or checks

I assume that with ./replaced/ you mean replaced/ in the directory in which the script is, not in the current working directory (as pwd); these are in general not the same. Please clarify.

Corrected, with other changes

use warnings;
use strict;
use feature qw(say);

use FindBin qw($RealBin);

use open ':std', ':encoding(UTF-8)';

my @files = grep { -f } @ARGV;    # add further checks of user input

my $outdir = "$RealBin/replaced";
mkdir $outdir if not -d $outdir;  # or use File::Path

foreach my $file (@files) {
    my $fout = "$outdir/$file";
    open my $fh, '<', $file or die "Can't open $file: $!";
    open my $fh_out, '>', $fout or die "Can't open $fout: $!";

    while (my $line = <$fh>) {
        $line =~ s/Insertstoredprocedure \( / Insertstoredprocedure('$file',/g;
        $line =~ s/SuccessSp\(\)/SuccessSp()('$file')/g;

        print $fh_out $line;
    }
    say "done, $file --> $fout";
}

Comments on code in the question

  • Always start programs with use warnings; and use strict;

  • The <*> reads all entries from the current directory, what poses difficult questions; for one, that may include the script itself. More importantly, that way your script is hardwired with what data to process. Why not take user input instead? I changed it to use what is submitted on the command line, filenames presumably. Then on Linux you can invoke the script as

    script.pl *.ext
    

    You can still use script.pl * if you must but then you need more checks, in particular to make sure to skip the script itself (if run from its directory). See for example this post

  • Always check input as appropriate. In this case you can at least ensure to process only plain files. I merely filter using -f filetest operator but another option is to take input as submitted and then check, so that you can inform the user about inadequate input

  • I see no need to introduce $filename; just use the topicalizer $file

  • If you work with UTF8 better use open pragma; then all files and streams are taken care of

  • Use lexical filehandles for everything, so for a file to be written as well

  • When reading a line from a file why not call it $line? The "$variable" in code is so generic that it provides no clue on what that variable is

  • Once you assigned to a named variable in the while condition then $_ not set to what is read; that happens only with while (<$fh>). In this code it is undefined inside the loop body. So in the regex you need to use that variable, to which the line is assigned (and not leave it to default $_)

  • Characters that have special meaning in regex must be escaped if you want to match them as literal characters, and parentheses are one of those. There are various ways to do that, I use your text and directly escape with \ (No need to escape in the replacement part)

  • It is in principle a good idea to define patterns as separate variables using qr operator. Then you can escape all special characters in them using quotemeta

I have no way to know whether your (corrected) regex does what is intended so I could only fix the obvious error. Please show samples of data and of needed output.

zdim
  • 64,580
  • 5
  • 52
  • 81
0

Could you please try with the following? I'm assuming that 'replaced' is found at current work directory.

use strict;
use warnings;
use Tie::File;
use English qw(-no_match_vars);

my @files = grep {-f} <*>;

-d './replaced/' or mkdir './replaced/';

foreach my $file (@files) {
    open my $fh, '<:encoding(UTF-8)', $file
      or die "Could not open file '$file': $OS_ERROR";
    open my $newfh, '>', "./replaced/$file"
      or die "Could not create new file './replaced/$file': $OS_ERROR";
    while (<$fh>) {
        s/Insertstoredprocedure\s*\(/Insertstoredprocedure('$file'/g;
        s/SuccessSp\s*\(/SuccessSp('$file'/g;
        print {$newfh} $_;
    }
    close $fh or die $OS_ERROR;
    close $newfh or die $OS_ERROR;
    print 'DONE with file: '.$file."\n";
}

Mandatory changes:

  1. filter (grep) <*> so that we discard directories. If not, you'll get a permission error trying to open a directory
  2. Scape (matching) parenthesis on regexes to \(
  3. Fix a bug on your code where you matched file line by line but you replaced $_ variable instead of $variable. Now It always work with $_
  4. Fixed last regex, It had some unneeded parenthesis
  5. You must print 'Done!' outside of the while loop, since that while is for every line.

Recommended changes:

  1. added use strict and use warnings (Very recommended)
  2. Use English and reference $! as $OS_ERROR
  3. Added my to the variable of the foreach
  4. Close files once read/written.
  5. Create 'replaced' folder if it does not exist
  6. For the output file, use lexical filehandle and 3-parameters open open my $newfh, '>', ...
Julio
  • 5,208
  • 1
  • 13
  • 42