Bash Shell Script uses Sed to create and insert multiple lines after a particular line in an existing file

Question

This code seems to work, but I'd like someone with shell-fu to review it.

• Is the temp file a good idea? How would I do without?
• Is sed the right tool?
• Any other general advice for improving my shell script

Script/Code to Review:

# Grab max field lengths from each .hbm.xml file and
# put them into the corresponding .java file
for myFile in $(find generatedSchema/myApp/db/ -name *.hbm.xml)
do
    # Calculate the java file name
    javaFile=${myFile/%\.hbm\.xml/.java}
    echo $javaFile

    # Find each field name and length and format it for the java file.
    # Save result lines to temp.txt
    sed -n '/<property name="[^"]\+" type="string">/{
        N
        s/ \+<property name="\([^"]\+\)" type="string">\n \+<column name="[^"]\+" length="\([0-9]\+\)".*/    @SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_\1 = \2;/p}' $myFile >temp.txt

    # Load results from temp file into a shell variable
    str=""
    while read line
    do
        echo $line
        str="$str\n    $line"
    done < temp.txt

    # Put new lines after the serialVersinUID line in the existing Java file.
    sed -i -e "s/^    private static final long serialVersionUID = [0-9]\+L;$/\0\n$str/" $javaFile

    # clean up
    rm temp.txt
done

Existing Input .hbm.xml file excerpt:

<?xml version="1.0"?>
<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN"
          "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd">
<hibernate-mapping>
    <class name="com.myCo.CodeReview" table="code_review" catalog="stackexchange">
        <!-- ... -->
        <property name="address1C" type="string">
            <column name="address1_c" length="50" />
        </property>
        <property name="address2C" type="string">
            <column name="address2_c" length="50" />
        </property>
        <!-- etc. -->

Existing Input Java File:

// ... preserve java code above

private static final long serialVersionUID = 20130318164201L;

// ... preserve java code below

Desired Output Java File:

// ... preserved java code above

private static final long serialVersionUID = 20130318164201L;

@SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_address1C = 50;
@SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_address2C = 50;

// ... preserved java code below

Background

This script generates Java objects that represent database tables. It modifies the output of the Hibernate Database Reverse-Engineering tools to add maximum field length tokens to my Java objects that match the length of those columns in the database.

Answer 1

1. Comments on your code

1. Your example input file is not a well-formed XML document. An XML document must have a single root element, but your example has two. What tool did you use to generate this file? You should ask yourself how it went wrong.

   (If you're just writing it "by hand" in some other program using a bunch of print statements, you might ask yourself why you are using XML at all rather than something simple like CSV. You clearly can't be intending to process these file using standard XML tools, since most XML tools will refuse to process ill-formed documents.)

2. In the find command:

   find generatedSchema/myApp/db/ -name *.hbm.xml
   

   the pattern *.hbm.xml would be expanded by the shell (rather than the find command) if there happened to be any files matching the pattern in the directory where this command is being run. So single-quote the pattern to make sure this can't happen:

   find generatedSchema/myApp/db/ -name '*.hbm.xml'
   

   You might also consider adding the argument -type f to ensure that the names returned all refer to regular files. (It's not very likely in your case that someone will name a directory foo.hbm.xml, but adding a -type argument to a find command is often good practice.)

3. In this line you don't need to escape the dots:

   javaFile=${myFile/%\.hbm\.xml/.java}
   

   The pattern in ${VAR/pat/rep} here is not a regular expression, so dots are not special. For example:

   $ FOO='a.b.c'
   $ echo ${FOO/./|}
   a|b.c
   

4. You use regular expressions to transform your XML into Java source. Now, as I'm sure you're aware, you can't reliably process XML using regular expressions. Perhaps what you've got here works because you control the format of these XML files. But really you should be using an XML-aware tool. See section 2 below.

5. You use a temporary file to store the output of your sed command, and then load it into a variable using a loop over its lines.

   You don't need this loop, because you can load the contents of a file into a variable by writing:

   VARIABLE=$(<filename)
   

   And you don't need the temporary file either, because you can load the output of a command into a variable by writing

   VARIABLE=$(command)
   

   So just write

   str=$(sed -n "...")
   

6. You could choose better variable names than myFile (why yours?) and str (what's in the string?).

7. In your last sed command, you use the expression s/pattern/\0\n$str/ to append $str after every match of pattern. You might consider using p;c instead so that you can simplify the pattern (it doesn't have to match the whole line any more):

   sed -i -e "/^private static final long serialVersionUID/{p;c\
   $str
   }"
   

8. You expand variables myFile and javaFile in your script in contexts like this:

   sed -i -e "..." $javaFile
   

   This will go wrong if the variable javaFile contains spaces. You should double-quote the variable to ensure that it's treated as a single argument to sed even if it contains spaces. (Perhaps you know in this case that it can't contain spaces, but double-quoting variables containing filenames is still a good habit to get into.)

   sed -i -e "..." "$javaFile"
   

9. I don't like the way you modify the Java source code in-place. This would be dangerous if this is your original Java source code, because a bug in your script could overwrite the Java source code with nonsense and break your build. (But since I can't see the rest of your build system I can't tell if this concern is justified.)

   If the whole Java file is automatically generated, wouldn't it be better to do it in one go rather than piecemeal like this? I have a feeling that there are more improvements to be made to this code generation process.

2. Using a programming language with XML support

If you need to process data from XML documents, then use a programming language together with an XML parsing library.

For example, here's how you might rewrite this same script in Python. First, you'd change the way you generate your .hbm.xml files so that they are well-formed XML documents. Let's suppose that they now look like this:

<properties>
  <property name="address1C" type="string">
    <column name="address1_c" length="50" />
  </property>
  <property name="address2C" type="string">
    <column name="address2_c" length="50" />
  </property>
</properties>

Then you might write the following Python program:

#!/usr/bin/python

import fileinput
import sys
import xml.etree.ElementTree

def main():
    # Check command-line argument.
    hbm_xml_filename = sys.argv[1]
    ext = '.hbm.xml'
    if not hbm_xml_filename.endswith(ext):
        raise RuntimeError("Filename {} doesn't end with {}"
                           .format(hbm_xml_filename, ext))

    # Load property names and maximum lengths from input file.
    tree = xml.etree.ElementTree.parse(hbm_xml_filename)
    properties = []
    for property in tree.findall('property[@type="string"]'):
        column = property.find('column')
        properties.append(dict(name = property.attrib['name'],
                               length = column.attrib['length']))

    # Rewrite Java in-place to add "MAX_name = length;" declarations.
    java_filename = hbm_xml_filename.replace(ext, '.java')
    for line in fileinput.input(files=[java_filename], inplace=True):
        sys.stdout.write(line)
        if line.startswith('private static final long serialVersionUID'):
            sys.stdout.write('\n')
            for p in properties:
                sys.stdout.write(
                    '@SuppressWarnings({{"UnusedDeclaration"}}) '
                    'public static final int MAX_{name} = {length};\n'
                    .format(**p))

if __name__ == '__main__':
    main()

You'll see that I've used the xml.etree.ElementTree module to parse and query the XML, I've used an XPath query to find all properties with the attribute type="string", and I've used the fileinput module to update the Java source code in-place.

Then you can run this Python program from your shell script:

find generatedSchema/myApp/db/ -type f -name '*.hbm.xml' |
while read property_file; do
    add_property_lengths.py "$property_file"
done

3. Addendum

You said you were having some difficulty getting VAR=$(command) to work. You should be able to verify that it works in general by testing it in the shell:

$ NUMBERS=$(yes '' | head | nl -ba)
$ echo $NUMBERS
1 2 3 4 5 6 7 8 9 10

Clearly we have different versions of sed (I'm using the BSD-ish version that comes with Mac OS X, so I had to specify -E for extended regular expressions and change the regular expression syntax a bit) but the following works for me:

VAR=$(sed -E -n '/ *<property name="([^"]+)" type="string">/{
    N
    s/ *<property name="([^"]+)" type="string">\n *<column name="[^"]+" length="([0-9]+)".*/    @SuppressWarnings({"UnusedDeclaration"}) public static final int MAX_\1 = \2;/
    p
}' "$myFile")

so you should be able to figure out how to make something like it work for you (if you must do it this way).