Perl script to handle include files

Question

I am trying to extend the JSON format (for private use) to include an include statement. This could be used to include other .json files into a master file, for example.

#! /usr/bin/perl

use warnings;
use strict;
use File::Slurp 'read_file';

my $str=parseFile('t.json');

sub parseFile {
  my ($fn)=@_;

  my $str=read_file($fn);
  my $res='';
  my $i=0;
  while (1) {
    if ($str=~/include\s+"(.*?)"/) {
      $res=$res.$`.parseFile("include/$1.json");
      $str=$';
    } else {
      $res=$res.$str;
      last;
    }
  }
  return $res;
}

It seems to be working for some simple test cases. But is it too complicated or is it too simple?

Answer 1

The $` and $' are very cryptic, I had to look them up in man perlvar to find that they contain whatever was before and after the match. It also turns out that they are discouraged due to performance considerations. Either way the while block really smells.

A simpler approach is to use s///ge to replace the matched pattern with the output of a function call, like this:

sub parseFile {
  my ($fn) = @_;
  my $str = read_file($fn);
  $str =~ s/include\s+"(.*?)"/parseFile("include\/$1.json")/ge;
  return $str;
}

Also consider adding some error handling. For example, what should happen if the file doesn't exist? Ignore / warn / raise error?

It might be a good idea to make parseFile handle multiple parameters:

sub parseFile {
    foreach my $fn (@_) {
        my $str = read_file($fn);
        $str =~ s/include\s+"(.*?)"/parseFile("include\/$1.json")/ge;
        return $str;
    }
}

Maybe you don't really need to handle multiple files, but a side effect of this approach is that it automatically handles the case gracefully if somebody incorrectly calls the method without any parameters.

Finally, notice that I used spaces around operators like = and =~ to improve readability. I borrow this practice from other programming languages where this is a standard, and let's face it, Perl scripts need all the readability aid they can get...

UPDATE

@amon added some excellent points in his comment:

• Notice that in the replacement pattern s/include\s+"(.*?)"/parseFile(".\/$1")/ge; I had to escape the delimiter /. Another way is changing the delimiter so I don't have to escape anything: s{include\s+"(.*?)"}{parseFile("include/$1")}ge;

• My multi-argument suggestion is kinda broken, because of the return statement: it will only process the first file. It would be better to use Carp::confess "parseFile expects one file name or file handle" if not @_ == 1, and the caller could still map { parseFile($_) } @files if really required