17
\$\begingroup\$

The following code transforms multi-line input into the the Brainfuck equivalent. One line turns into one program. The resulting programs are split by newlines for readability.

Brainf.cpp

#include "Brainf.hpp"

int main()
{
    std::vector<std::string> userInput;
    std::string currentLine = " "; // do not start empty

    while (currentLine.length() != 0) // escape input by an empty line
    {
        std::getline(std::cin, currentLine);
        userInput.push_back(currentLine);
    }

    std::vector<std::string>::size_type userInputSize = userInput.size();

    for (unsigned i = 0; i < userInputSize; i++)
    {
        std::cout << modifyString(userInput[i]) << std::endl << std::endl;
    }
}

Brainf.hpp

#ifndef BRAINF_HPP
#define BRAINF_HPP

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

std::string modifyChar(char input, int carry)
{
    std::string output;
    int next = input - carry;
    if (next > 0)
    {
        for (unsigned i = 0; i < next; ++i)
        {
            output += "+";
        }
    }
    if (next < 0)
    {
        for (unsigned i = 0; i < abs(next); ++i)
        {
            output += "-";
        }
    }
    output += ".";

    return output;
}

std::string modifyString(std::string input)
{
    std::string output;
    int traveller = 0;
    for (unsigned i = 0; i < input.length(); ++i)
    {
        output += modifyChar(input[i], traveller);
        traveller = (int)input[i];
    }
    return output;
}

#endif

Explanation:

The print statement in Brainfuck is .. The resulting Brainfuck prints the original string character-by-character, so there will be as much . as there are characters to print. Between those . there will be either + if the ASCII is higher than the previous character and - if it's lower.

The Brainfuck doesn't think. It increments, decrements or prints.

Example usage:

Input:

Hello World!

Output:

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++
++++++++++++++++++++++.+++++++..+++.--------------------------------------------
-----------------------------------.++++++++++++++++++++++++++++++++++++++++++++
+++++++++++.++++++++++++++++++++++++.+++.------.--------.-----------------------
--------------------------------------------.

Input:

FizzBuzz
Foo!Bar!

Output:

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++++
++++++++++++++++++++++++++.+++++++++++++++++..----------------------------------
----------------------.+++++++++++++++++++++++++++++++++++++++++++++++++++.+++++
..

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++++
++++++++++++++++++++++++++++++++..----------------------------------------------
--------------------------------.+++++++++++++++++++++++++++++++++.+++++++++++++
++++++++++++++++++.+++++++++++++++++.-------------------------------------------
--------------------------------------.

Points of concern:

  • I'm bad at naming things.
  • My user-input handling feels bad.
  • There's a cast in my code which feels like a design flaw.
  • I may be needlessly repeating myself in one case while not splitting responsibilities far enough in another.
  • It's not modular.
  • It may not be idiomatic C++11.
  • It may not be safe (although IMHO, there's not much which can go wrong).
\$\endgroup\$
8
\$\begingroup\$

First of all, let's not beat around the bush. Clearly your filenames should be Brainfuck.hpp and Brainfuck.cpp. Let's not be shy. Now for the real reviews:

Input

std::string currentLine = " "; // do not start empty

while (currentLine.length() != 0) // escape input by an empty line
{
    std::getline(std::cin, currentLine);
    userInput.push_back(currentLine);
}

This is bad. getline() could fail - and you'd expect it to fail when the input runs out. But you're not checking the result of getline(), so you end up writing an extra string onto your vector. The correct approach would be:

for (std::string currentLine; std::getline(std::cin, currentLine); ) {
    userInput.push_back(currentLine);
}

This ensures that you get the correct strings in your userInput.

Looping

Since this is tagged C++11, you definitely want to use a range-for expression to do your loop. It's less code and clearer:

for (const std::string& str : userInput) {
    std::cout << modifyString(str) << std::endl << std::endl;
}

Know what's in the standard

Your modifyChar() function's job is to pick some character (either + or -) and return a string with that character next times, followed by a period. You do not need a loop for this. There is a constructor for std::string that takes a count and a character and constructs a string with that character that many times.

Thus:

std::string modifyChar(char input, int carry)
{
    int next = input - carry;
    std::string output = (next > 0) ? std::string(next, '+')
                                    : std::string(-next, '-');
    return output + '.';
}

And same comment about looping again for modifyString. Though additionally you should take input by reference-to-const, to avoid a completely unnecessary copy:

std::string modifyString(std::string const& input)
{
    std::string output;
    int traveller = 0;
    for (char c : input)
    {
        output += modifyChar(c, traveller);
        traveller = c;
    }
    return output;
}
\$\endgroup\$
  • \$\begingroup\$ I knew I was doing something the wrong way around with my input. These are all very valuable suggestions! \$\endgroup\$ – Mast Jul 22 '15 at 23:48
4
\$\begingroup\$

Similar code:

if (next > 0)
{
    for (unsigned i = 0; i < next; ++i)
    {
        output += "+";
    }
}
if (next < 0)
{
    for (unsigned i = 0; i < abs(next); ++i)
    {
        output += "-";
    }
}

These two snippets are almost identical. I would combine them like this:

char toAppend = next > 0 ? '+' : '-';

for (unsigned i = 0; i < abs(next); ++i)
{
    output += toAppend;
}

Also, I would probably just use an int unless you really need the larger number size provided by an unsigned int, in which case I would probably use a long.


Char to Int Cast

traveller = (int)input[i];

When you assigned a char to an int, you do not need to cast. This two statements are the same (although the explicit cast shows you are deliberately doing it):

traveller = (int)input[i];
traveller = input [i];
\$\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.