Return to Revisions

Usability

How the f.[< do I use this interpreter when it doesn't come with a main() function? Here's the simplest implementation I came up with, using java.nio.file.*:

public static void main(String[] args) throws IOException {
 String code = new String(Files.readAllBytes(Paths.get(args[0])));
 // TODO: Implement InputStreamToByteIteratorAdaptor
 BrainF interp = createFromCodeAndInput(DEFAULT_MEMORY_SIZE, code, new InputStreamToByteIteratorAdaptor());
 interp.runToEnd();
 System.out.println(interp.getOutput());
}

Even that shows some internal usability problems. I would have expected

public static void main(String[] args) throws IOException {
 (new BrainF(new File(args[0]))).run();
}

to work. Notably,

• The factory method name doesn't need to mention "default" — BrainF.create() or new BrainF() would work just as well. Let me worry about the size when I want to worry about the size. The interpreter can also auto-expand the memory as needed.
• Specifying the input via an Iterator<Byte> and buffering the output to a StringBuilder is extremely cumbersome. It should be easy to hook up the input and output to System.in and System.out, respectively — in fact, that is how it should be unless the I/O is deliberately redirected. I shouldn't have to write an InputStreamToByteIteratorAdaptor to run an interactive program.
• runToEnd() could just be run().

Additionally, this factory method is problematic:

public static BrainF createFromCodeAndInput(int memorySize, String code, Stream<Byte> inputStream) {
 return new BrainF(DEFAULT_MEMORY_SIZE, code, inputStream);
}

It is buggy in that it discards its memorySize paramater. Also, it serves the same purpose as the BrainF(memorySize, code, in) constructor. If you offer factory methods, then make the constructor private to avoid cluttering the interface offerings. Alternatively, just offer multiple constructors.

Commands

The giant switch in BrainF.perform() is a code smell. Why bother creating an enum at all, when you could just switch on character literals directly? Alternatively, if you do have an enum, then the behaviour of each command should be implemented in the command itself.

SUBSTRACT should be renamed SUBTRACT. (Personally, I would have chosen "increment"/"decrement".)