Output the most used non-trivial words from a file

Question

For fun I'm solving some university homework assignments, and here is one that I was lucky enough to stumble upon. Basically, here are all the requirements (modified a bit from the original because some points are nonsensical):

Write a program that reads a text file and an amount of words from the command line, and outputs the most used words with more than 4 characters in the text file along with their frequency.

You should convert every word to lowercase. Every word is a sequence of letters, and every other character is a separator.

I wanted to use the (hopefully) upcoming ranges from C++20, but because there is no implementation of them in a Standard Library yet, I used C++17 and the range-v3 library.

I couldn't get the official test file, and so I had to make my own. I'm not sure if I'm allowed to upload it though. Every suggestion from naming to performance to code quality is welcome.

#include <fstream>
#include <iterator>
#include <iostream>
#include <range/v3/all.hpp>
#include <vector>
#include <algorithm>
#include <string>

auto group() noexcept {
    return ranges::view::group_by([](auto&& lhs, auto&& rhs) {
            return std::isalpha(lhs) == std::isalpha(rhs);
        })
        | ranges::view::transform([](auto&& list) {
            return ranges::accumulate(list, std::string{});
        });
}

auto remove_separators() noexcept {
    return ranges::view::filter([](auto&& str) {
        return std::all_of(str.begin(), str.end(), [](auto&& c) {
            return std::isalpha(c);
        });
    });
}

auto restrict() noexcept {
    return ranges::view::filter([](auto&& str) {
        return str.length() >= 5 && std::all_of(str.begin(), str.end(), [](auto&& c) {
            return std::isalpha(c);
        });
    });
}

auto lowercase() noexcept {
    return ranges::view::transform([](auto str) {
        std::transform(str.begin(), str.end(), str.begin(), [](auto&& c) {
            return std::tolower(c);
        });
        return str;
    });
}

auto map() noexcept {
    return ranges::view::group_by([](auto&& lhs, auto&& rhs) {
            return lhs == rhs;
        })
        | ranges::view::transform([](auto&& list) {
            return std::pair(list | ranges::view::take(1), ranges::distance(list));
        });
}

int main(int argc, char* argv[]) noexcept {
    if (argc != 3) {
        std::cout << "Usage: ./" << argv[0] << " [file] [word-amount]\n";
        return -1;
    }

    std::ifstream file{argv[1]};
    if (!file.is_open()) {
        std::cout << "Couldn't open file " << argv[1] << "!\n";
        return -1;
    }

    auto amount = 0u;
    try {
        amount = std::stoi(argv[2]);
    }
    catch (const std::exception&) {
        std::cout << "Invalid number " << argv[2] << "!\n";
    }

    std::vector<char> character_list;
    char character = '\0';
    while (file.read(&character, 1))
        character_list.push_back(character);

    auto words = character_list | group() | remove_separators()
        | restrict() | lowercase() | ranges::to_<std::vector>() | ranges::action::sort;

    auto highlights = words | map() | ranges::to_<std::vector>()
        | ranges::action::sort([](auto&& lhs, auto&& rhs) {
            return lhs.second > rhs.second;
        });

    for (auto&& word : highlights | ranges::view::take(amount))
        std::cout << word.first << ": " << word.second << '\n';
    return 0;
}

Answer 1

We're missing an include of <cctype> for std::isalpha and std::tolower. We also have a bug in how we use these functions - remember that they accept positive integers, not (possibly signed) chars.

So instead of

[](auto&& c) {
    return std::isalpha(c);
}

(which could have simply been written std::isalpha), we need to make the chars unsigned before widening:

[](unsigned char c) {
    return std::isalpha(c);
}

---

remove_separators() is inefficient because it tests using std::all_of(). If we know we'll be applying this to the result of group(), then we know each string is all-alpha or all-nonalpha, so we just need to test the first character (and we know there's no empty strings after group()).

---

We use quite a lot of non-range algorithms where the range equivalents are simpler to use. For example:

    std::transform(str.begin(), str.end(), str.begin(), [](auto&& c) {
        return std::tolower(c);
    });

becomes

ranges::for_each(str, [](unsigned char c){ return std::tolower(c); });

---

if (!file.is_open()) {
    std::cout << "Couldn't open file " << argv[1] << "!\n";
    return -1;
}

Error messages should go to std::cerr. And it's better to return small positive values from main() (preferably EXIT_FAILURE, from <cstdlib>).

---

catch (const std::exception&) {
    std::cout << "Invalid number " << argv[2] << "!\n";
}

Probably a good idea to exit in this case, too. I don't think we should be continuing with a default amount.

---

Reading the file a character at a time into a vector seems crazy when much better options exist. Rather than reading the whole file into memory, we could reduce our resource consumption by processing a line at a time (since newlines are whitespace) and accumulating the resulting histograms.