-3
\$\begingroup\$

I wrote a Brainfuck interpreter in JavaScript, however it is quite buggy and I can't figure out what I'm doing wrong. It works for programs I've written, but fails on most programs I find on the internet. It seems that the while loop doesn't work correctly when it is supposed to skip over. When debugging, the value of i (index) behaves as I intended. In addition, I feel that my code is overly complicated for such a simple task, and I think there might be a way to simplify my code, although I can't think of a way :(

Here is the code:

function execBrainf() {
  document.getElementById("output").value = "";
  var ptr = 0, i, ii;
  var cells = new Array(), labels = new Array();
  for (i = 0; i < 30000; ++i)
    cells[i] = 0;
  for (i = ii = 0; i < document.getElementById("code").value.length; ++i) {
    switch (document.getElementById("code").value.charAt(i)) {
      case '>':
        ++ptr; break;
      case '<':
        --ptr; break;
      case '+':
        cells[ptr] = (++cells[ptr] % 256); break;
      case '-':
        cells[ptr] = (--cells[ptr] % 256); break;
      case ',':
        cells[ptr] = (ii > document.getElementById("input").length ? 0 : document.getElementById("input").value.charCodeAt(ii++)); break;
      case '.':
        document.getElementById("output").value = document.getElementById("output").value + String.fromCharCode(cells[ptr]); break;
      case '[':
        (cells[ptr] == 0 ? i = document.getElementById("code").value.indexLoopEnd(i) : labels.push(i)); break;
      case ']':
        (cells[ptr] == 0 ? labels.pop() : i = labels[labels.length - 1]); break;
    }


  }
}

function indexLoopEnd(i) {
  var x = 1;
  while (x > 0) {
    switch (this.charAt(++i)) {
      case '[':
        ++x; break;
      case ']':
        --x; break;
    }
  }
    return i;
}

I am using the html here: http://esotools.ml/brainfuck/interpreter.html

I am looking for:
- what the problem is
- suggestions on making the code prettier
- suggestions for improving the algorithm

I am not too concerned about optimization yet, and would like to focus on these three things.

\$\endgroup\$
1

1 Answer 1

3
\$\begingroup\$

Store document.getElement(s)By... calls

You call the method document.getElementById quite a lot in this code, and it's all to get the same exact elements: "input", "output", and "code"

While it is a wonderfully useful method, it is also expensive so it's best to reduce the calls to it as much as possible.

function execBrainf() {
  var input = document.getElementById("input");
  var output = document.getElementById("output");
  var code = document.getElementById("code");

Flexibility

The above tip is somewhat linked to this.

Right now, your code works only a very specific environment: there must be three elements with specific IDs and specific information in them else this is not going to work.

Yes, I understand, you wrote this for that specific environment, but we can still make the code more easy to test if we instead have the function take the code, input, and output through parameters:

function execBf(code, input) {

Then, you could easily test this code by passing in strings as the code and input, and have the function return the output (thanks to Pieter Witvoet for proposing a better idea than functions)


Simplify setting the cells to 0

This is borrowing from a very good answer I once read.

Rather than iterating 30000 times to initialize an array's values to 0, you can define a function that will return 0 for a cell's value based on its index if the cell's value is undefined (which is the default value for JavaScript array values).

function getCell(i) {
    return cells[i] || 0;
}

Now you don't have to go through that long loop.


Misc.

  • Create new arrays with [], not new Array()

  • ....value.indexLoopEnd(i) Is that a bug/mistype/mis-version? You only defined indexLoopEnd as a function.

\$\endgroup\$
5
  • \$\begingroup\$ Thanks! I intended for indexLoopEnd to be a method returning an integer.. How would I define a method? Or am I doing it completely wrong? I am trying to assign i to the corresponding close bracket ']' by calling indexLoopEnd(i), which returns the index of the close bracket. \$\endgroup\$ Apr 7, 2016 at 0:57
  • 1
    \$\begingroup\$ You could technically make it into a method by overriding the Number.prototype but I couldn't think of a worse thing to do in JavaScript (well, yes I can, but you get the idea). I think it is better as a standalone function. \$\endgroup\$
    – SirPython
    Apr 7, 2016 at 1:00
  • \$\begingroup\$ Thanks!! I have updated it. It works on fizzbizz now, but for some reason mandelbrot set still crashed my program... \$\endgroup\$ Apr 7, 2016 at 14:46
  • 1
    \$\begingroup\$ Personally I would pass code and input as strings, and I'd make execBrainf return the output as a string, instead of passing in html elements. That completely decouples it from any specific UI. Useful if, for example, you want to run benchmarks (where showing output is not relevant). Or if you want to run automated tests (without UI). \$\endgroup\$ Apr 7, 2016 at 19:39
  • 1
    \$\begingroup\$ @PieterWitvoet Thanks for the tip! I've updated my post. \$\endgroup\$
    – SirPython
    Apr 7, 2016 at 20:01

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