Skip to main content
Code Review

Return to Answer

added 412 characters in body
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

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 {
 BrainFString interpcode = BrainF.createWithDefaultSize();
  interp.addCommands(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.
  • Accepting the code as a Stream<Byte> is not that useful. How often do you append more Brainfuck on-the-fly to be interpreted? It's not a programming technique I've ever used.
  • On the other hand, streaming the input and output would be useful. SpecifyingSpecifying 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".)

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 {
 BrainF interp = BrainF.createWithDefaultSize();
  interp.addCommands(new String(Files.readAllBytes(Paths.get(args[0]))));
 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.
  • Accepting the code as a Stream<Byte> is not that useful. How often do you append more Brainfuck on-the-fly to be interpreted? It's not a programming technique I've ever used.
  • On the other hand, streaming the input and output would be useful. 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().

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".)

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".)

Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

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 {
 BrainF interp = BrainF.createWithDefaultSize();
 interp.addCommands(new String(Files.readAllBytes(Paths.get(args[0]))));
 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.
  • Accepting the code as a Stream<Byte> is not that useful. How often do you append more Brainfuck on-the-fly to be interpreted? It's not a programming technique I've ever used.
  • On the other hand, streaming the input and output would be useful. 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().

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".)

lang-java

AltStyle によって変換されたページ (->オリジナル) /