3
\$\begingroup\$

This is basically a program that takes the first line of a file and makes a map using ggGgg h gGgGg s makes ggGgg equals h and gGgGg equals s. Now the rest of the file makes sentences with these. Idea is from here.

#include <string>
#include <sstream>
#include <fstream>
#include <iostream>
#include <map>
#include <stdexcept>
#include <algorithm>
#include <cctype>

std::map<std::string, char> makeMap(std::ifstream&);
const std::string makeWord(const std::string&, const std::map<std::string, char>&);

void translate(std::ifstream &file) {
    auto transMap = makeMap(file);
    std::string line;
    while (std::getline(file, line)) {
        bool firstWord = true;
        std::string word;
        std::istringstream wordIN(line);
        while (wordIN >> word) {
            if (firstWord)
                firstWord = false;
            else
                std::cout << " ";
            std::cout << makeWord(word, transMap);
        }
        std::cout << "\n";
    }

}

std::map<std::string, char> makeMap(std::ifstream &toTrans) {
    std::map<std::string, char> transMap;

    std::string line;
    std::getline(toTrans, line);
    std::istringstream input(line);

    std::string key;
    char value;

    while (input >> key) {
        if (key.find_first_not_of("Gg") != std::string::npos)
            throw std::runtime_error("You can only use g's");
        if (key.size() != 5)
            throw std::runtime_error("The key is not equal to 5");

        if (input >> value)
            transMap[key] = value;
        else
            throw std::runtime_error("No value for the key");
    }
    return transMap;
}

const std::string makeWord(const std::string &word, const std::map<std::string, char> &m) {
    std::string oldWord, newWord;
    for (auto letter = word.cbegin(); letter != word.cend(); ++letter) {
        if (!std::ispunct(*letter)) {
            oldWord.push_back(*letter);
                if (oldWord.size() == 5) {
                    if (m.find(oldWord) != m.cend()) {
                        newWord.push_back((m.find(oldWord)->second));
                        oldWord.clear();
                    }
                    else 
                        throw std::runtime_error("Couldnt find word");
                }
            }
        else
            newWord.push_back(*letter);
    }
    return newWord;
}

int main()
{
    std::ifstream file("ReadMe.txt");
     translate(file);
    return 0;
}

My "ReadMe.txt" file is:

GggGg e GGGgg o GGggg l  ggGgg h GGGGg o GgGgg r gGgGg l GGgGg d ggggg w
ggGggGggGgGGgggGGgggGGGGg! gggggGGGGgGgGgggGgGgGGgGg!
ggGggGggGgGGgggGGgggGGGGg gggggGGGGgGgGgggGgGgGGgGg
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Handle singular cases outside of the loop

The firstWord variable is there to handle a single word, and complicates the whole loop (and makes it a tiny bit slower, but that is secondary):

while (std::getline(file, line)) {
    bool firstWord = true;
    std::string word;
    std::istringstream wordIN(line);
    while (wordIN >> word) {
        if (firstWord)
            firstWord = false;
        else
            std::cout << " ";
        std::cout << makeWord(word, transMap);
    }
    std::cout << "\n";
}

I would write it like:

while (std::getline(file, line)) {
    std::string word;
    std::istringstream wordIN(line);

    wordIN >> word;
    std::cout << makeWord(word, transMap);
    while (wordIN >> word) {
        std::cout << " ";
        std::cout << makeWord(word, transMap);
    }
    std::cout << "\n";
}

This way the code is more simple because the special case is clearly handled alone out of the loop.

Algorithm / IO separation

All your functions (except main) are void, so their only purpose is modifying the outside world. This code is mainly about solving a conceptual (algoritmhic) problem, so it would benefit greatly if logic and Input Output were separated, because testing would become much easier.

\$\endgroup\$

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