C++ range-v3: trying to chain together transforms

Question

I'm completely new to the range library, so I shouldn't be surprised that this code isn't compiling and I cannot figure out why:

#include <iostream>
#include <algorithm>
#include <fstream>
#include <iterator>
#include <vector>

#include <range/v3/all.hpp>
#include <range/v3/view/all.hpp>
using namespace ranges::v3;


std::ifstream open_file(const std::string &filename) {
    return std::ifstream{filename};
}

int count_lines(std::ifstream &in) {
    return std::count(std::istreambuf_iterator<char>{in},
                      std::istreambuf_iterator<char>{}, '\n');
}

std::vector<int>
count_lines_in_files(const std::vector<std::string> &filenames) {
    auto a1 = filenames | view::transform(open_file) | view::transform(count_lines);
    return a1;
}

int main() {
    const std::vector<std::string> files{"listing1_1.cpp",
                                         "listing1_2.cpp",
                                         "listing1_4.cpp",
                                         "listing1_5.cpp"};
    const auto result = count_lines_in_files(files);
    std::cout << ranges::view::all(result) << '\n';
}

It appears that the complaint is about a1, which the compiler tells me "error: variable has incomplete type 'void'."

Can someone see what I'm doing wrong, or tell me how to properly chain these together if possible?

Thanks in advance!

When I try compiling this, there are more error messages after that, including a static_assert which attempts to explain exactly what's wrong. This won't compile because `count_lines(open_file(*begin(files)))` won't compile. — aschepler, May 24 '19 at 04:01

score 3 · Answer 1 · answered May 24 '19 at 09:11

3

std::ifstream doesn't have a copy constructor - returning std::ifstream by a function is not a good idea. One possible solution: opening and counting should take place in one function.

answered May 24 '19 at 09:11

Porsche9II

629
5
17

This was actually in a book, Functional Programming in C++ by Manning, in that they specifically did it in one function as you suggest but then recommend splitting it into two functions as per my question. Now that I think about it, you're absolutely right: it seems like a bad idea to return an `ifstream` from a function. – Sebastian May 24 '19 at 21:06

Bob__ · Accepted Answer · 2019-05-24T14:43:49.640

3

As noted by Porsche9II, "std::ifstream doesn't have a copy constructor". You can find more on this topic here:

Why are iostreams not copyable?

C++11 introduced a move constructor (6) for std::basic_ifstream, so you could write

auto open_file(const std::string &filename) {
    return std::ifstream{filename};
}

auto count_lines(std::ifstream &&in) {
    return std::count(std::istreambuf_iterator<char>{in},
                      std::istreambuf_iterator<char>{}, '\n');
}

Testable HERE.

edited May 24 '19 at 14:43

answered May 24 '19 at 09:51

Bob__

12,361
3
28
42

1

Nice one - better than my solution :-) and the warnings go away with gcc 9.1.0 ... – Porsche9II May 24 '19 at 14:33
1

@Porsche9II Thanks, also removing the offending `::v3` works, with Clang. – Bob__ May 24 '19 at 14:45
Excellent. Thanks very much for this. This is from the book, Functional Programming in C++ by Manning (who published the famous Functional Programming in Scala book). The author makes the suggestion to split the code from one function opening a file and reading lines to the two function approach I attempted. @Porsche9II makes me wonder, though: is it a good idea to be passing file streams around? Is there any inherent risk in doing so instead of containing them to one function? (The book's author also says all his code is in gitlab, but it's severely lacking. Unimpressed so far.) – Sebastian May 24 '19 at 21:09
1

@Sebastian In the Q&A I linked, you can find this [comment](https://stackoverflow.com/questions/36051672/why-are-iostreams-not-copyable#comment59751947_36051672) with a quote from Scott Meyers about the reasons why the copy constructor isn't implemented. Move semantic should solve those issues, but I'm still not quite familiar with ranges library to give valuable tips, I'm afraid. – Bob__ May 24 '19 at 21:28

C++ range-v3: trying to chain together transforms

2 Answers2