5
\$\begingroup\$

Description:

I wrote a Java program that checks whether the content of the given file begins with the Java class File Format magic number 0xCAFEBABE, in big-endian byte order.

It prints a success message if:

  • The first 4 bytes match the magic number

It prints an error message if:

  • The file doesn't exist or cannot be read
  • The file contains less than 4 bytes
  • The first 4 bytes don't match the magic number

It prints a usage message if:

  • A file is not given as input

Code:

import java.io.*;

public class Magic {

    public static void main(String[] args) {
        if (args.length < 1) {
            System.out.println("Usage: java Magic <FILE>");
            return;
        }

        try {
            if (isMagicPresent(args[0])) {
                System.out.println(String.format("'%s' contains the magic number.", args[0]));
            } else {
                System.out.println(String.format("'%s' doesn't contain the magic number.", args[0]));
            }
        } catch (IOException ex) {
            System.err.println(ex.getMessage());
        }
    }

    private static boolean isMagicPresent(String file) throws IOException {
        try (InputStream instream = new FileInputStream(file)) {
            byte[] bytes = new byte[4];
            int readCount = instream.read(bytes);

            if (readCount == 4) {
                return Byte.toUnsignedInt(bytes[0]) == 0xCA &&
                        Byte.toUnsignedInt(bytes[1]) == 0xFE &&
                        Byte.toUnsignedInt(bytes[2]) == 0xBA &&
                        Byte.toUnsignedInt(bytes[3]) == 0xBE;
            }
        }
        return false;
    }

}

Question:

How can I improve the above code in terms of:

  • Readability
  • Performance
  • Boundary case and error handling
\$\endgroup\$
3
\$\begingroup\$

Interface

For clarity, isMagicPresent() should accept a java.io.File (or java.nio.file.Path) to emphasize that the argument is a path and not the contents of a file.

The default constructor should be suppressed.

Implementation

To read a big-endian int, use DataInputStream.readInt(). Then you can compare the magic number to 0xCAFEBABE rather than four bytes.

Driver

I recommend returning a non-zero exit code to indicate that an error has occurred.

When no argument is given, the error message should go to System.err. (If you supported a -h or --help option, then its output should go to System.out, because that would be the requested output.)

Instead of System.out.println(String.format(…)), you should just call System.out.format(…). Here, I've chosen to combine both cases into one print statement, but you don't have to.

Suggested implementation

import java.io.*;

public class Magic {
    private Magic() {}

    public static boolean isMagicPresent(File f) throws IOException {
        try (InputStream inStream = new FileInputStream(f);
             DataInputStream dataInStream = new DataInputStream(inStream)) {
            return dataInStream.readInt() == 0xCAFEBABE;
        }
    }

    public static void main(String[] args) {
        if (args.length < 1) {
            System.err.println("Usage: java Magic <FILE>");
            System.exit(1);
        }

        File f = new File(args[0]);
        try {
            System.out.format(
                "'%s' %s the magic number.%n",
                f,
                isMagicPresent(f) ? "contains" : "doesn't contain"
            );
        } catch (IOException ex) {
            System.err.println(ex.getMessage());
            System.exit(1);
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

For me overall it was very easy to read and follow.

Nitpicking:

  • Since you want to accept a single input, you can actually check for if (args.length == 1)

  • You are already chaining the conditionals so why not the following?

private static boolean isMagicPresent(String file) throws IOException {
    try (InputStream is = new FileInputStream(file)) {
        byte[] bytes = new byte[4];
        return is.read(bytes) == 4 &&
                Byte.toUnsignedInt(bytes[0]) == 0xCA &&
                Byte.toUnsignedInt(bytes[1]) == 0xFE &&
                Byte.toUnsignedInt(bytes[2]) == 0xBA &&
                Byte.toUnsignedInt(bytes[3]) == 0xBE;
    }
}
  • You can present the example usage within quotes as in 'filename.ext' since whitespaces are allowed in most operating systems and I may try running your program as in java Magic Mag ic.class which would return Mag (No such file or directory) but running with java Magic 'Mag ic.class' returns 'Mag ic.class' contains the magic number. when I actually have this file..

  • isMagicPresent can accept an InputStream instead of a file path which would make testing the method easier and de-coupling it from a file.

private static boolean isMagicPresent(InputStream is) throws IOException {
    byte[] bytes = new byte[4];
    return is.read(bytes) == 4 &&
            Byte.toUnsignedInt(bytes[0]) == 0xCA &&
            Byte.toUnsignedInt(bytes[1]) == 0xFE &&
            Byte.toUnsignedInt(bytes[2]) == 0xBA &&
            Byte.toUnsignedInt(bytes[3]) == 0xBE;
}

Keep up the good work.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.