7
\$\begingroup\$

In a followup this application was turned into a GUI using FLTK: Cleaning a file / Word Query GUI (FLTK)

I did the following two exercises in Programming: Principles and Practice Using C++ (2nd Edition), by Stroustrup, which build upon each other:

From Chapter 21 (Algorithms and Maps),

  1. Write a program to "clean up" a text file for use in a word query program; that is, replace punctuation with whitespace, put words into lower case, replace don't with do not (etc.), and remove plurals (e.g ships becomes ship). Don't be too ambitious. For example, it is hard to determine plurals in general, so just remove an s if you find both ship and ships. Use that program on a real-world text file with at least 5000 words (e.g., a research paper).

  2. Write a program (using the output from the previous exercise) to answer questions such as:

    • "How many occurrences of ship are there in a file?"
    • "Which word occurs most frequently?"
    • "Which is the longest word?"
    • "Which is the shortest?"
    • "List all words starting with s."
    • “List all four-letter words."

I solved this tasks as follows:

Cleaned_words.h

#ifndef CLEAN_FILE290320180702_GUARD
#define CLEAN_FILE290320180702_GUARD

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

namespace cleaned_words {

    using Word = std::string;
    using Occurences = int;

    std::map<Word, Occurences> read_words_from_file(const std::string& filename);
    std::map<Word, Occurences> read_cleaned_words_with_occurence(std::istream& is);
    bool contains_digits(const Word& word);
    Word remove_invalid_signs(const Word& word, const std::string& invalid_signs);

    inline unsigned char unsigned_isspace(char c)
    {
        return isspace(static_cast<unsigned char>(c));
    }

    Word remove_whitespace(const Word& word);
    Word remove_capital_letters(const Word& word);
    std::vector<Word> remove_contractions(const Word& word);
    void remove_plural(std::map<Word, Occurences>& cleaned_words);
    void write_cleaned_words_to_file(const std::string& filename, const std::map<Word, Occurences>& cleaned_words);
}
#endif

Cleaned_words.cpp

#include "Cleaned_words.h"

#include <algorithm>
#include <cctype>
#include <fstream>

namespace cleaned_words {

    std::map<Word, Occurences> read_words_from_file(const std::string& filename)
    {
        std::ifstream ifs{ filename };
        if (!ifs) {
            throw std::runtime_error("void read_words_from_file(const std::string& filename)\nFile could not be opened\n");
        }

        return read_cleaned_words_with_occurence(ifs);
    }

    std::map<Word, Occurences> read_cleaned_words_with_occurence(std::istream& is)
    {
        std::map<Word, Occurences> cleaned_words;

        for (Word word; is >> word;) {

            if (contains_digits(word)) continue;

            word = remove_invalid_signs(word, R"(°-_^@{}[]<>&.,_()+-=?“”:;/\")");
            word = remove_whitespace(word);
            word = remove_capital_letters(word);

            if (word.empty()) continue;

            std::vector<Word> words = remove_contractions(word);

            for (auto& word : words) {  // remove ' after concatenations were run to not erase them to early
                word = remove_invalid_signs(word, "'");
            }

            for (const auto& word : words) {

                if (word.size() == 1 && word != "a" && word != "i" && word != "o") continue;

                ++cleaned_words[word];
            }
        }

        remove_plural(cleaned_words);

        return cleaned_words;
    }

    bool contains_digits(const Word& word)
    {
        if (word.empty()) return false;

        for (const auto&x : word) {                             //erase digits etc
            if (isdigit(static_cast<unsigned char>(x))) {
                return true;
            }
        }
        return false;
    }

    Word remove_invalid_signs(const Word& word,const std::string& invalid_signs)
        // replace invalid signs with whitespace
    {
        Word cleaned_word = word;
        for (auto it = cleaned_word.begin(); it != cleaned_word.end();)
        {
            if (std::find(invalid_signs.begin(), invalid_signs.end(), *it) != invalid_signs.end()) {
                it = cleaned_word.erase(it);
            }
            else{
                ++it;
            }
        }

        return cleaned_word;
    }

    Word remove_whitespace(const Word& word)
    {
        if (word.empty()) return word;

        Word cleaned_word = word;

        cleaned_word.erase(std::remove_if(cleaned_word.begin(), cleaned_word.end(), unsigned_isspace), cleaned_word.end());     

        return cleaned_word;
    }

    Word remove_capital_letters(const Word& word)
    {
        Word clean_word = word;

        for (auto& letter : clean_word) {
            letter = std::tolower(letter);
        }
        return clean_word;
    }

    std::vector<Word> remove_contractions(const Word& word)
    {
        const std::map<Word, std::vector<Word>> shorts_and_longs
        {
            { "aren't",{ "are","not" }},
            { "can't",  {"cannot"} },
            { "could've",{ "could","have" } },
            { "couldn't",{ "could","not" } },
            { "daresn't",{ "dare","not" } },
            { "dasn't",{ "dare","not" } },
            { "didn't",{ "did","not" } },
            { "doesn't",{ "does","not" } },
            { "don't",{ "do","not" } },
            { "e'er",{ "ever" } },
            { "everyone's",{ "everyone","is" } },
            { "finna",{ "fixing","to" } },
            { "gimme",{ "give","me" } },
            { "gonna",{ "going","to" } },
            { "gon't",{ "go","not" } },
            { "gotta",{ "got","to" } },
            { "hadn't",{ "had","not" } },
            { "hasn't",{ "has","not" } },
            { "haven't",{ "have","not" } },
            { "he've",{ "he","have" } },
            { "how'll",{ "how","will" } },
            { "how're",{ "how","are" } },
            { "I'm",{ "I","am" } },
            { "I'm'a",{ "I","am","about","to" } },
            { "I'm'o",{ "I","am","going","to" } },
            { "I've",{ "I","have" } },
            { "isn't",{ "is","not" } },
            { "it'd",{ "it","would" } },
            { "let's",{ "let","us" } },
            { "ma'am",{ "madam" } },
            { "mayn't",{ "may","not" } },
            { "may've",{ "may","have" } },
            { "mightn't",{ "might","not" } },
            { "might've",{ "might","have" } },
            { "mustn't",{ "must","not" } },
            { "mustn't've",{ "must","not","have" } },
            { "must've",{ "must","have" } },
            { "needn't",{ "need","not" } },
            { "ne'er",{ "never" } },
            { "o'clock",{ "of","the","clock" } },
            { "o'er",{ "over" } },
            { "ol'",{ "old" } },
            { "oughtn't",{ "ought","not" } },
            { "shan't",{ "shall","not" } },
            { "should've",{ "should","have" } },
            { "shouldn't",{ "should","not" } },
            { "that're",{ "that","are" } },
            { "there're",{ "there","are" } },
            { "these're",{ "these","are" } },
            { "they've",{ "they","have" } },
            { "those're",{ "those","are" } },
            { "'tis",{ "it","is" } },
            { "'twas",{ "it","was" } },
            { "wasn't",{ "was","not" } },
            { "we'd've",{ "we","would","have" } },
            { "we'll",{ "we","will" } },
            { "we're",{ "we","are" } },
            { "we've",{ "we","have" } },
            { "weren't",{ "were","not" } },
            { "what'd",{ "what","did" } },
            { "what're",{ "what","are" } },
            { "what've",{ "what","have" } },
            { "where'd",{ "where","did" } },
            { "where're",{ "where","are" } },
            { "where've",{ "where","have" } },
            { "who'd've",{ "who","would","have" } },
            { "who're",{ "who","are" } },
            { "who've",{ "who","have" } },
            { "why'd",{ "why","did" } },
            { "why're",{ "why","are" } },
            { "won't",{ "will","not" } },
            { "would've",{ "would","have" } },
            { "wouldn't",{ "would","not" } },
            { "y'all",{ "you","all" } },
            { "y'all'd've",{ "you","all","would","have" } },
            { "yesn't",{ "yes","not" } },
            { "you're",{ "you","are" } },
            { "you've",{ "you","have" } },
            { "whomst'd've",{ "whomst","would","have" } },
            { "noun's",{ "noun","is" } },
        };

        auto it = shorts_and_longs.find(word);

        if (it == shorts_and_longs.end()) {
            return std::vector<Word>{word};
        }
        else {
            return it->second;
        }

        return std::vector<Word>{};
    }

    void remove_plural(std::map<Word, Occurences>& cleaned_words)
        // assume a plural is a word with an additional s
        // e.g. ship and ships
        // if both are present ships gets deleted and ++ship
    {
        for (auto it = cleaned_words.begin(); it != cleaned_words.end();) {

            if(!it->first.empty() && it->first.back() == 's') {
                Word singular = it->first;
                singular.pop_back();    // remove 's' at the end

                auto it_singular = cleaned_words.find(singular);

                if (it_singular != cleaned_words.end()) {
                    cleaned_words[it_singular->first]+= it->second;
                    it = cleaned_words.erase(it);
                }
                else {
                    ++it;
                }
            }
            else {
                ++it;
            }
        }
    }

    void write_cleaned_words_to_file(const std::string& filename, const std::map<Word, Occurences>& cleaned_words)
    {
        std::ofstream ofs{ filename };

        for (const auto& word : cleaned_words) {
            ofs << word.first << " " << word.second << '\n';
        }
    }
}

Word_query.h

#ifndef WORD_QUERY_GUARD_270820181433
#define WORD_QUERY_GUARD_270820181433

#include <map>
#include <optional>
#include <string>
#include <vector>

namespace word_query {
    using Word = std::string;
    using Occurences = int;
    using Length = std::map<Word, Occurences>::size_type;

    int occurences_of_word(const Word& word, const std::map<Word, Occurences>& words_with_occurences);
    std::optional<std::pair<Word, Occurences>> most_frequent_word(const std::map<Word, Occurences>& words_with_occurences);
    std::optional<Word> longest_word(const std::map<Word, Occurences>& words_with_occurences);
    std::optional<Word> shortest_word(const std::map<Word, Occurences>& words_with_occurences);
    std::vector<Word> words_starting_with(const Word& begin_of_word, const std::map<Word, Occurences>& words_with_occurences);
    std::vector<Word> words_with_length(Length length, const std::map<Word, Occurences>& words_with_occurences);
}
#endif

Word_query.cpp

#include "Word_query.h"

#include <algorithm>

namespace word_query {

    int occurences_of_word(const Word& word, const std::map<Word, Occurences>& words_with_occurences)
        //How many occurences of x are there in a file? 
    {
        auto it = words_with_occurences.find(word);

        if (it == words_with_occurences.end()) {
            return 0;
        }
        else {
            return it->second;
        }
    }

    std::optional<std::pair<Word, Occurences>> most_frequent_word(const std::map<Word, Occurences>& words_with_occurences)
        //Which word occurs most frequently?
    {
        if (words_with_occurences.empty()) return std::nullopt;

        using pair_type = std::map<Word, Occurences>::value_type;

        auto most_frequent = std::max_element(
            words_with_occurences.begin(), words_with_occurences.end(),
            [](const pair_type a, const pair_type b)
            {
                return a.second < b.second;
            }
        );

        if (most_frequent == words_with_occurences.end()) {
            return std::nullopt;
        }
        else {
            return std::optional<std::pair<Word, Occurences>>{*most_frequent};
        }
    }

    std::optional<Word> longest_word(const std::map<Word, Occurences>& words_with_occurences)
        //Which is the longest word in the file?
    {
        if (words_with_occurences.empty()) return std::nullopt;

        using pair_type = std::map<Word, Occurences>::value_type;

        auto most_frequent = std::max_element(
            words_with_occurences.begin(), words_with_occurences.end(),
            [](const pair_type a, const pair_type b)
            {
                return a.first.size() < b.first.size();
            }
        );

        if (most_frequent == words_with_occurences.end()) {
            return std::nullopt;
        }
        else {
            return std::optional<Word>{most_frequent->first};
        }
    }

    std::optional<Word> shortest_word(const std::map<Word, Occurences>& words_with_occurences)
        //Which is the shortest word in the file?
    {
        if (words_with_occurences.empty()) return std::nullopt;

        using pair_type = std::map<Word, Occurences>::value_type;

        auto most_frequent = std::min_element(
            words_with_occurences.begin(), words_with_occurences.end(),
            [](const pair_type a, const pair_type b)
            {
                return a.first.size() < b.first.size();
            }
        );

        if (most_frequent == words_with_occurences.end()) {
            return std::nullopt;
        }
        else {
            return std::optional<Word>{most_frequent->first};
        }
    }

    std::vector<Word> words_starting_with(const Word& begin_of_word, const std::map<Word, Occurences>& words_with_occurences)
    {
        std::vector<Word> matched_words;
        for (const auto& word : words_with_occurences) {
            if (word.first.substr(0, begin_of_word.size()) == begin_of_word) {
                matched_words.push_back(word.first);
            }
        }
        return matched_words;
    }

    std::vector<Word> words_with_length(Length length, const std::map<Word, Occurences>& words_with_occurences)
        //all words with n letters
    {
        if (length < 0) {
            throw std::runtime_error(
                "std::vector<Word> words_with_length(Length length, const std::map<Word, Occurences>& words_with_occurences)\nlength must be positive\n");
        }

        std::vector<Word> words;

        for (const auto& element : words_with_occurences) {
            if (element.first.size() == length)
                words.push_back(element.first);
        }
        return words;
    }
}

main.cpp

#include <exception>
#include <iostream>

#include "Cleaned_words.h"
#include "Word_query.h"

int main()
try{
    std::cout << "Enter filename:\n";
    std::string filename;
    std::cin >> filename;

    auto words = cleaned_words::read_words_from_file(filename);
    cleaned_words::write_cleaned_words_to_file( "out_" + filename , words);

    std::cout << "Enter word to search occurences for:\n";
    std::string occur_word;
    std::cin >> occur_word;

    std::cout   << occur_word << " is present in " 
                << filename << " " 
                << word_query::occurences_of_word(occur_word, words) 
                << " times\n";

    auto most_frequent = word_query::most_frequent_word(words);
    if (most_frequent) {
        std::cout   << "The most frequent word in " << filename 
                    << " is: " << most_frequent->first 
                    << " with " << most_frequent->second 
                    << " occurences\n";
    }

    auto longest = word_query::longest_word(words);
    if (longest) {
        std::cout << "The longest word in " << filename
            << " is: " << *longest << '\n';
    }

    auto shortest = word_query::shortest_word(words);
    if (shortest) {
        std::cout << "The shortest word in " << filename
            << " is: " << *shortest << '\n';
    }

    std::cout << "Enter begining of words:\n";
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::string word_begining;
    std::cin >> word_begining;

    std::cout << "All words starting with " << word_begining << " present in " << filename << '\n';
    auto words_starting_with = word_query::words_starting_with(word_begining, words);
    for (const auto& word : words_starting_with) {
        std::cout << word << '\n';
    }

    std::cout << "Enter word length:\n";
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    auto length = 0;
    std::cin >> length;

    std::cout << "All words with length of " << length << " present in " << filename << '\n';
    auto words_with_specific_length = word_query::words_with_length(length, words);
    for (const auto& word : words_with_specific_length) {
        std::cout << word << '\n';
    }

    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::cin.get();
}
catch (std::runtime_error& e) {
    std::cerr << e.what() << "\n";
    std::cin.get();
}
catch (...) {
    std::cerr << "unknown error " << "\n";
    std::cin.get();
}

Besides main, where all the functions can be tested on a real file and asking the user for decision, I also thought about testing the single functions independently. For that I dug into unit tests with googletest in MSVC2017. I found out MSVC by default allready has the option to set up a test project which can link to the code.

For more details about this, see this and this.

When getting the unit tests to run I run into the issue that Google tests seem to use still stuff from std::tr1 which leads to issues if MSVC is turned to C++17. The strange solution Google gave and which has worked was adding /Zc:__cplusplus on the command line in the test project.

Beside that MSVC generated a file test.cpp from which I could run all my added unit tests linked to the project code.

test.cpp

#include "pch.h"

#include "../PP_CH21_EX13_clean_txt/Cleaned_words.h"
#include "../PP_CH21_EX13_clean_txt/Word_query.h"

#include <sstream>

namespace cleaned_words {

    TEST(contains_digits_test, detect_digits)
    {
        ASSERT_FALSE(cleaned_words::contains_digits("hello"));
        ASSERT_TRUE(cleaned_words::contains_digits("1234hello"));
        ASSERT_TRUE(cleaned_words::contains_digits("hello1234"));
        ASSERT_TRUE(cleaned_words::contains_digits("hel1lo"));
    }

    TEST(remove_invalid_signs_test, Invalid_sign_was_removed)
    {
        ASSERT_EQ("Hello", remove_invalid_signs(".,_()+-=?“”:;\"Hello.,_()+-=?“”:;\"", R"({}[]<>&.,_()+-=?“”:;/\")"));
        ASSERT_EQ("a", remove_invalid_signs("\"a", R"({}[]<>&.,_()+-=?“”:;/\")"));
    }

    TEST(remove_whitespace_test, whitespace_gets_removed)
    {
        ASSERT_EQ("", remove_whitespace(""));
        ASSERT_EQ("Hello", remove_whitespace("Hello"));
        ASSERT_EQ("Hello", remove_whitespace("       H   e l       l   o       "));
    }

    TEST(remove_capital_letters_test, capital_letters_get_lowered)
    {
        ASSERT_EQ("", remove_capital_letters(""));
        ASSERT_EQ("hellohello", remove_capital_letters("HELLOhello"));
    }

    TEST(remove_contractions_test, words_get_transformed_to_long_form)
    {
        std::vector<Word> a = { "are", "not" };
        std::vector<Word> b = remove_contractions("aren't");

        ASSERT_EQ(a.size(), b.size());

        for (std::vector<Word>::size_type i = 0; i < a.size(); ++i) {
            ASSERT_EQ(a[i], b[i]);
        }
    }

    TEST(remove_plural_test, plural_gets_removed)
    {
        int c1 = 10;
        int c2 = 5;;

        std::map<Word, Occurences> test{
            {"ship",c1},
            {"ships",c2}
        };

        remove_plural(test);

        ASSERT_TRUE(test.find("ships") == test.end());
        ASSERT_TRUE(test.find("ship") != test.end());

        ASSERT_TRUE(test.find("ship")->second == c1 + c2);

        ASSERT_TRUE(test.size() == 1);
    }


    TEST(read_cleaned_words_with_occurence_test, words_get_cleaned)
    {
        std::string s = "HELLO hello this is a ship ships ships do not now 123test aren't";
        std::istringstream ifs{ s };

        auto res = read_cleaned_words_with_occurence(ifs);

        ASSERT_TRUE(res["hello"] == 2);
        ASSERT_TRUE(res["ship"] == 3);
        ASSERT_TRUE(res.find("ships") == res.end());
        ASSERT_TRUE(res.find("aren't") == res.end());
        ASSERT_TRUE(res.find("are") != res.end());
        ASSERT_TRUE(res.find("not") != res.end());
    }
}

namespace word_query {

    TEST(occurences_of_word_test, correct_occurence)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14}
        };

        ASSERT_TRUE(occurences_of_word("hello", words) == 20);
        ASSERT_TRUE(occurences_of_word("this", words) == 14);
        ASSERT_TRUE(occurences_of_word("is", words) == 0);
    }

    TEST(most_frequent_word_test, find_most_frequent)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14},
            {"is",98},
            {"a",3},
            {"test",5}
        };

        ASSERT_EQ("is",(most_frequent_word(words))->first);
    }

    TEST(longest_word_test, find_longest_word)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14},
            {"is",98},
            {"a",3},
            {"test",5}
        };

        ASSERT_EQ("hello", longest_word(words));
    }

    TEST(shortest_word_test, find_shortest_word)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14},
            {"is",98},
            {"a",3},
            {"test",5}
        };

        ASSERT_EQ("a", shortest_word(words));
    }

    TEST(words_starting_with_test, find_words_starting_with)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14},
            {"is",98},
            {"a",3},
            {"test",5}
        };

        std::vector<Word> res;

        res = words_starting_with("th", words);
        ASSERT_EQ(1, res.size());
        ASSERT_EQ("this", res[0]);

        res = words_starting_with("t", words);
        ASSERT_EQ(2, res.size());
    }


    TEST(words_with_length_test, find_words_with_length)
    {
        std::map<Word, Occurences> words
        {
            {"hello",20},
            {"this",14},
            {"is",98},
            {"a",3},
            {"test",5}
        };


        std::vector<Word> res;

        res = words_with_length(1, words);
        ASSERT_EQ(1, res.size());
        ASSERT_EQ("a", res[0]);

        res = words_with_length(2, words);
        ASSERT_EQ(1, res.size());
        ASSERT_EQ("is", res[0]);

        res = words_with_length(3, words);
        ASSERT_TRUE(res.empty());

        res = words_with_length(4, words);
        ASSERT_TRUE(2, res.size());
        ASSERT_EQ("this", res[1]);
        ASSERT_EQ("test", res[0]);
    }
}

Please let me know what you think about the presented code.

  1. Are there any improvements which could be done to solve these exercises?
  2. Are there any bad practices in the code? Is there stuff made to complicated? Is there an easier solution?
  3. How would you improve the tests?
\$\endgroup\$
6
\$\begingroup\$
inline unsigned char unsigned_isspace(char c)
{
    return isspace(static_cast<unsigned char>(c));
}

Misspelt std::isspace, and forgot to include <cctype>.

A simpler implementation would just use the argument type to convert c:

inline bool unsigned_isspace(unsigned char c)
{
    return std::isspace(c);
}

    std::ifstream ifs{ filename };
    if (!ifs) {
        throw std::runtime_error("void read_words_from_file(const std::string& filename)\nFile could not be opened\n");
    }

Perhaps std::filesystem_error?


    if (isdigit(static_cast<unsigned char>(x))) {

std::isdigit

Instead of writing a loop, you could use std::any_of algorithm.


for (auto it = cleaned_word.begin(); it != cleaned_word.end();)
{
    if (std::find(invalid_signs.begin(), invalid_signs.end(), *it) != invalid_signs.end()) {
        it = cleaned_word.erase(it);
    }
    else{
        ++it;
    }
}

Why not erase-remove, as in remove_whitespace immediately following? You could implement it like this:

Word remove_invalid_signs(Word word,const std::string& invalid_signs)
        // replace invalid signs with whitespace
    {
        auto is_invalid = [&](char c){ return invalid_signs.find(c) != std::string::npos; };
        word.erase(std::remove_if(word.begin(), word.end(), is_invalid), word.end());
        return word;
    }

(Note: I changed word from a const-ref to a value, since we only used it to copy from).


    for (auto& letter : clean_word) {
        letter = std::tolower(letter);
    }

We need to launder letter through unsigned char, as we do for std::isspace() and std::isdigit().


In each of most_frequent_word(), longest_word() and shortest_word() we have both:

    if (words_with_occurences.empty()) return std::nullopt;

And:

    if (most_frequent == words_with_occurences.end()) {
        return std::nullopt;

We need only one of those tests!


Removed: the return type is an array of a different kind to the input type; we need a range adapter to turn a map's iterator into an iterator of its keys.

In words_starting_with():

    for (const auto& word : words_with_occurences) {
        if (word.first.substr(0, begin_of_word.size()) == begin_of_word) {
            matched_words.push_back(word.first);
        }
    }

That loop could be a std::copy_if(); similarly for words_with_length().


    if (length < 0) {
        throw std::runtime_error(
            "std::vector<Word> words_with_length(Length length, const std::map<Word, Occurences>& words_with_occurences)\nlength must be positive\n");
    }

length is an unsigned type, so cannot be less than 0.


The common code in the functions that return a vector of words could be refactored out:

template<typename Pred>
std::vector<Word> words_matching(const std::map<Word, Occurences>& words_with_occurences,
                                 const Pred& predicate)
{
    std::vector<Word> matched_words;
    for (auto const& x: words_with_occurences) {
        if (predicate(x.first)) {
            matched_words.push_back(x.first);
        }
    }
    return matched_words;
}

std::vector<Word> words_starting_with(const Word& begin_of_word, const std::map<Word, Occurences>& words_with_occurences)
{
    return words_matching(words_with_occurences, [&begin_of_word](const Word& word) {
            return word.compare(0, begin_of_word.size(), begin_of_word, 0) == 0;
        });
}

std::vector<Word> words_with_length(Length length, const std::map<Word, Occurences>& words_with_occurences)
    //all words with n letters
{
    return words_matching(words_with_occurences, [&length](const Word& word) {
            return word.size() == length;
        });
}

    std::vector<Word> a = { "are", "not" };
    std::vector<Word> b = remove_contractions("aren't");

    ASSERT_EQ(a.size(), b.size());

    for (std::vector<Word>::size_type i = 0; i < a.size(); ++i) {
        ASSERT_EQ(a[i], b[i]);
    }

std::vector has a working == operator, so that test can be simply

    ASSERT_EQ(a, b);

    ASSERT_TRUE(2, res.size());

I think that should be

    ASSERT_EQ(2, res.size());
\$\endgroup\$
  • \$\begingroup\$ Thanks for the detailed answer. I have trouble reworking remove_invalid_signs with erase_remove like you suggested. Also i don't understand how i can rework words_starting_with() and words_with_length with copy_if. I would appreachiate if you could give an example of that. \$\endgroup\$ – Sandro4912 Aug 29 '18 at 12:01
  • \$\begingroup\$ I was wrong about copy_if - input is a map not a vector. It would be possible with a range adapter to convert the map's iterator into an iterator of its keys, but I think that's overkill here. I have added a suggestion to extract the conversion from these, and a demonstration of how remove_invalid_signs can utilise erase-remove idiom. \$\endgroup\$ – Toby Speight Aug 29 '18 at 12:38
  • \$\begingroup\$ BTW, thanks for including the unit tests with your code - that made it much easier to try out changes. Much appreciated. \$\endgroup\$ – Toby Speight Aug 29 '18 at 16:43
2
\$\begingroup\$

While adding a GUI to this exercise, I found a logical error.

These functions:

std::optional<std::pair<Word, Occurences>> most_frequent_word(const std::map<Word, Occurences>& words_with_occurences);
std::optional<Word> longest_word(const std::map<Word, Occurences>& words_with_occurences);
std::optional<Word> shortest_word(const std::map<Word, Occurences>& words_with_occurences);

all currently return one pair / word or nothing. This is wrong: there can be files with a joint longest, shortest or modal word.

I changed all three functions to return a collection of results:

std::vector<std::pair<Word, Occurences>> most_frequent_words(const std::map<Word, Occurences>& words_with_occurences);
std::vector<Word> longest_words(const std::map<Word, Occurences>& words_with_occurences);
std::vector<Word> shortest_words(const std::map<Word, Occurences>& words_with_occurences);

Now, std::optional is not needed any more: we can simply check for an empty vector.

\$\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.