5
\$\begingroup\$

This question is the adaptation of this question in C++. (I'm the author of that code as well).

The code:

#include <bits/stdc++.h>

std::string bf_interpreter(std::string code){
    long long int ptr = 0;

    std::vector<int> values = {0};
    long long int length = 1;

    std::vector<int> brackets;

    std::string result = "";

    for(unsigned long long int index = 0; index < code.size(); index++){
        while(length <= ptr){
            length++;
            values.push_back(0);
        }

        char character = code[index];

        switch(character){
            case '>': ptr++; break;
            case '<': ptr--; break;

            case '+': values[ptr] = (values[ptr] + 1) % 256; break;
            case '-': values[ptr] = (values[ptr] - 1) % 256; break;

            case '[':
                brackets.push_back(index);
                break;

            case ']':
                if(values[ptr] == 0)
                    brackets.pop_back();
                else
                    index = brackets[brackets.size() - 1];

                break;

            case '.':
                result += (char)values[ptr];
                break;

            case ',':
                char input; std::cin >> input;
                values[ptr] = (int)input;
                break;
        }
    }

    return result;
}


int main()
{
    std::string s;

    std::cin >> s;

    std::cout << bf_interpreter(s);
}

I'm quite a beginner in C++, so please excuse me if I'm not following any standard conventions.

\$\endgroup\$
5
\$\begingroup\$

Bug

The only issue I see is that the data array is never expanded.

As a result any value of ptr that is not zero will cause undefined behavior. What you want to do is give a fixed but reasonable size for the data.

std::size_t const maxDataSize = 10000;

...

   std::vector<int> values;
   values.resize(maxDataSize);

Then make sure when you access the data you mod it to the correct range:

        case '>': ptr = (ptr + 1) % maxDataSize; break;
        case '<': ptr = (ptr - 1) % maxDataSize; break;

Comment

This is not a standard C++ header.

#include <bits/stdc++.h>

What you want is

#include <vector>
#include <string>
#include <iostream>

Comment

The code is non modifiable so the the code parameter should be constant. I would also pass by const reference to prevent a copy or require a move operation to pass the code into the interpretor.


I can see how you would interpret the output as what is printed to the output stream by the interpreter as the result. But is not the whole of memory the actual output?


Two things I would do:

  1. Make . output to the output stream (like , takes from the input stream.

  2. I would give the bf_interpreter() two parameters. The code to execute and the data memory that can be used by the program. The data memory is passed by reference so that any changes can be inspected after the interpreter has finished.

  3. Potentially I would even pass two streams (input/output) as parameters to bf_interpreter(). Though that is a bit more subjective.

Thus my function definition would be this:

 std::string bf_interpreter(std::string const& code,
                            std::vector<int>&  memory,
                            std::istream&      input = std::cin,
                            std::ostream&      output = std::cout
                           );

ReDesign

OK. So given there is a lot more state here than I really want to pass as a function. I would change this a bit and create a "Brain Fuck" class where all the important state is passed to the constructor then you can call interpret on the object.

class BF_Engine
{
         // Add Members here
         void nextStep() {
             // Add interpreter here
         }
    public:
         BF_Engine(std::string const& code,
                   std::vector<int>&  memory,
                   std::istream&      input = std::cin,
                   std::ostream&      output = std::cout
                  );

         void interpret(std::size_t steps = 1)
         {
             while (!finished && steps > 0) {
                 nextStep();
                 --steps;
             }
         }
         void interpretRun()
         {
             while (!finished) {
                 nextStep();
             }
         }
 };
\$\endgroup\$
2
\$\begingroup\$

Naming

Functions do things, so the name for a function should normally be a verb. In your case, the function should be something like bf_interpret, not ...interpreter.

I'd also at least consider making bf a namespace, so the call in main would be something like:

bf::interpret(s);

Prefix operators

Lacking a reason to do otherwise, I'd prefer to use prefix operators rather than postfix.

     case '>': ++ptr; break;
     case '<': --ptr; break;

Use the right types

Since you're restricting the items in values to the range 0..255 anyway, I'd at least consider making them uint8_t, which guarantees exactly that range. This lets you simplify the code somewhat:

        case '+': ++values[ptr]; break;
        case '-': --values[ptr]; break;

If you wanted the values to act differently, I'd still just use increment and decrement here, and implement the desired wrapping (or whatever) behavior in a separate data type, and make values a vector of that data type.

Input/Output

While I think Martin York is on the right track in saying that the input to and output from interpretation should be passed as parameters, I'd disagree about the exact form of those parameters. Rather than streams, I'd make them iterators, whose exact types are specified as template parameters. A user who wants to read/write streams for the I/O can do so by passing stream iterators. At the same time, a user who wants to read from a vector and write to a string (or whatever) can do that as well.

\$\endgroup\$
  • \$\begingroup\$ I like iterators at first glance, but the interface to streams is easier concept to pass. As iterators require a beginning and end for each stream. On the other hand it would be simple to provide a stream buffer that wraps two iterators giving you the advantage of the iterator but using the stream interface. \$\endgroup\$ – Martin York Dec 3 at 18:21

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.