Skip to main content
Error-handling problems
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481

Minor issues

head -n -2 is not portable. The head in GNU Coreutils supports a negative number, but it's not POSIX.

If you create temporary files, use mktemp(1), so that you won't accidentally overwrite an existing file that is coincidentally has the same name as what you chose for the temp file. Also, immediately set a trap handler to clean up properly, even if the program aborts or is interrupted by ControlC.

If you detect a fatal condition (input file doesn't exist), bail out immediately. Don't put the error handler at the end, far from where you detected the problem. You would also avoid a level of indentation for nearly the entire script.

The error message should be printed to standard error, to avoid contaminating standard output. The exit status of your script should be non-zero in case of an error.

Single-language approach

Mixing Bash, AWK, head, and tail is rarely pretty. Why not write the whole program in AWK alone?

#!/usr/bin/env awk -f

BEGIN {
    OFS = "\t";
    ELEMENT_FOR_ATOMIC_WEIGHT["12.011150"] = "C";
    ELEMENT_FOR_ATOMIC_WEIGHT["1.007970"] = "H";
}

/^$/ {
    next;               # Ignore all blank lines
}

$2 == "atoms" {
    print $1
}

/Masses/, /Pair Coeffs/ {
    ATOMIC_WEIGHT_FOR_ATOM_TYPE[$1] = $2
}

/Atoms/, /Bonds/ {
    print ELEMENT_FOR_ATOMIC_WEIGHT[ATOMIC_WEIGHT_FOR_ATOM_TYPE[$3]], $5, $6, $7
}

Minor issues

head -n -2 is not portable. The head in GNU Coreutils supports a negative number, but it's not POSIX.

If you create temporary files, use mktemp(1), so that you won't accidentally overwrite an existing file that is coincidentally has the same name as what you chose for the temp file. Also, immediately set a trap handler to clean up properly, even if the program aborts or is interrupted by ControlC.

If you detect a fatal condition (input file doesn't exist), bail out immediately. Don't put the error handler at the end, far from where you detected the problem. You would also avoid a level of indentation for nearly the entire script.

Single-language approach

Mixing Bash, AWK, head, and tail is rarely pretty. Why not write the whole program in AWK alone?

#!/usr/bin/env awk -f

BEGIN {
    OFS = "\t";
    ELEMENT_FOR_ATOMIC_WEIGHT["12.011150"] = "C";
    ELEMENT_FOR_ATOMIC_WEIGHT["1.007970"] = "H";
}

/^$/ {
    next;               # Ignore all blank lines
}

$2 == "atoms" {
    print $1
}

/Masses/, /Pair Coeffs/ {
    ATOMIC_WEIGHT_FOR_ATOM_TYPE[$1] = $2
}

/Atoms/, /Bonds/ {
    print ELEMENT_FOR_ATOMIC_WEIGHT[ATOMIC_WEIGHT_FOR_ATOM_TYPE[$3]], $5, $6, $7
}

Minor issues

head -n -2 is not portable. The head in GNU Coreutils supports a negative number, but it's not POSIX.

If you create temporary files, use mktemp(1), so that you won't accidentally overwrite an existing file that is coincidentally has the same name as what you chose for the temp file. Also, immediately set a trap handler to clean up properly, even if the program aborts or is interrupted by ControlC.

If you detect a fatal condition (input file doesn't exist), bail out immediately. Don't put the error handler at the end, far from where you detected the problem. You would also avoid a level of indentation for nearly the entire script.

The error message should be printed to standard error, to avoid contaminating standard output. The exit status of your script should be non-zero in case of an error.

Single-language approach

Mixing Bash, AWK, head, and tail is rarely pretty. Why not write the whole program in AWK alone?

#!/usr/bin/env awk -f

BEGIN {
    OFS = "\t";
    ELEMENT_FOR_ATOMIC_WEIGHT["12.011150"] = "C";
    ELEMENT_FOR_ATOMIC_WEIGHT["1.007970"] = "H";
}

/^$/ {
    next;               # Ignore all blank lines
}

$2 == "atoms" {
    print $1
}

/Masses/, /Pair Coeffs/ {
    ATOMIC_WEIGHT_FOR_ATOM_TYPE[$1] = $2
}

/Atoms/, /Bonds/ {
    print ELEMENT_FOR_ATOMIC_WEIGHT[ATOMIC_WEIGHT_FOR_ATOM_TYPE[$3]], $5, $6, $7
}
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481

Minor issues

head -n -2 is not portable. The head in GNU Coreutils supports a negative number, but it's not POSIX.

If you create temporary files, use mktemp(1), so that you won't accidentally overwrite an existing file that is coincidentally has the same name as what you chose for the temp file. Also, immediately set a trap handler to clean up properly, even if the program aborts or is interrupted by ControlC.

If you detect a fatal condition (input file doesn't exist), bail out immediately. Don't put the error handler at the end, far from where you detected the problem. You would also avoid a level of indentation for nearly the entire script.

Single-language approach

Mixing Bash, AWK, head, and tail is rarely pretty. Why not write the whole program in AWK alone?

#!/usr/bin/env awk -f

BEGIN {
    OFS = "\t";
    ELEMENT_FOR_ATOMIC_WEIGHT["12.011150"] = "C";
    ELEMENT_FOR_ATOMIC_WEIGHT["1.007970"] = "H";
}

/^$/ {
    next;               # Ignore all blank lines
}

$2 == "atoms" {
    print $1
}

/Masses/, /Pair Coeffs/ {
    ATOMIC_WEIGHT_FOR_ATOM_TYPE[$1] = $2
}

/Atoms/, /Bonds/ {
    print ELEMENT_FOR_ATOMIC_WEIGHT[ATOMIC_WEIGHT_FOR_ATOM_TYPE[$3]], $5, $6, $7
}