7
\$\begingroup\$

I'm writing a library of IO functions for my physics laboratory class. In the meanwhile, I'm hoping to learn more about generic programming and C++20 concepts.

Some context

I usually came home from the lab with a .txt file that looks like

value value value ... value value value ... 

where each line represents a set of repeated measurements of the same physical quantity.

I want to write a function that reads one line at a time from my .txt and put all the values it founds into some vector. My idea is to mock getline

The actual code

#include <algorithm> // for std::transform #include <fstream> // for std::ifstream #include <iterator> // for std::istream_iterator #include <sstream> // for std::istringstream #include <string> // for std::string and std::getline #include <type_traits> // for std::is_arithmetic_v #include <vector> // for std::vector 
template<typename T> concept Arithmetic = std::is_arithmetic_v<T>; 
namespace sf { namespace io { template <Arithmetic T> std::vector<T> getline(std::ifstream& data) { std::vector<T> meas; std::string line{}; while (line.size() == 0 && !data.eof()) { std::getline(data, line); } std::istringstream lines{line}; std::istream_iterator<std::string> linei{lines}; std::transform(linei, std::istream_iterator<std::string>{}, std::back_inserter(meas), [] (std::string s) {return std::stod(s);}); return meas; } } } // namespace io // namespace sf 

I.e., I first define a new concept for "arithmetic" types, and then I write a generic function that

  1. eats a file
  2. tries reading a line until we've got to an actual nonempty line or to eof
  3. parses whatever it got, and puts it into a std::vector.

I'm planning to use my code like this

std::string path = "path/to/data.txt" std::ifstream file(path); auto meas1 = sf::io::getline<double>(file); auto meas2 = sf::io::getline<double>(file); // and so on 

What I already know my code does not achieve

  1. First of all, I would like to make my code less dependent on the vector class.
  2. At some point I use the standard library's stod function. There is at least another way of parsing number literals from strings to actual arithmetic types... but am I on the right path? I read that one could achieve exactly what I am trying to achieve, in a more idiomatic way, by the means of the >> operator...
\$\endgroup\$

    1 Answer 1

    6
    \$\begingroup\$

    Naming things

    I would avoid using exactly the same name as the function from the standard library. Yours does a bit more. Maybe get_values_from_line() or get_line_values() would be better.

    Avoid unnecessary abbreviations, or if you do, use commonly used ones. So instead of meas write measurements (or perhaps just result), and instead of linei write line_iterator or line_it.

    lines sounds like it is the plural of line, but it still is just a single line. Maybe this is an abbreviation of line_stream?

    Pass a std::istream instead of a std::ifstream

    Your getline() function doesn't care whether the stream is a file or any other kind of stream. So make it take a reference to a std::istream. That way, something like this will also compile:

    auto values = sf::io::getline<double>(std::cin); 

    Opportunities to improve performance

    The way you wrote the code, after reading in a line into a string, you copy that string into a string stream object. It's better to move it:

    std::istringstream line_stream{std::move(line)}; 

    Next, you are creating a stream iterator that returns strings, and then you call std::stod() to convert those strings to T. But why not let the stream iterator return Ts instead?

    std::istream_iterator<T> line_it{lines}; std::copy(line_it, std::istream_iterator<T>{}, std::back_inserter(measurements) ); 

    Error handling

    Unfortunately, many things can go wrong when doing I/O: there might be a read error reading a file, or the file may contain something that is not just correctly formatted numbers. You should make your code robust against all these things, and instead of returning a std::vector that contains nonsense, make sure you return an error to the caller somehow.

    The first issue is that data.eof() wil return false if an error occurs before reaching the actual end of the file. Subsequent calls to std::getline() will likely not fix the error, so the while-loop will then become an infinite loop. The best thing to do is check for data.good():

    while (data.good() && !line.empty) { std::getline(data, line); } 

    Then, consider that std::istream_iterator may encounter errors as well. If that happens, check lines.good(). If it returns false, then you know that the line was not read correctly.

    There are various options for handling errors:

    You could also return an empty vector, as normally you ignore empty lines and that would thus be an unexpected output. However, that is too easy to ignore in the rest of your code.

    Make it more generic

    I already mentioned using a reference to a std::istream instead of std::ifstream, this makes the code usable in many more situations. But once you use std::istream_iterator<T>, you'll notice that you can put in any type that operator>>() supports. So if you want a vector of chars or std::strings, that will now also work. Consequently, the constraint that T is Arithmetic is more narrow than necessary.

    Also consider that you might not want to store the results in a std::vector, but rather in a std::deque, std::list or some other container. It would be nice if you could avoid hardcoding the result type. The best way would be to make your function return a view of the elements of type T of a single line, and then the caller can do whatever they want with that. Consider being able to write:

    auto values = sf::io::get_line_view<double>(file) | std::ranges::to<std::vector>(); 

    Or perhaps the caller didn't want to store it in a container anyway, but just iterate over the values, then they could write:

    for (auto value: sf::io::get_line_view<double>(file)) { … } 

    One way to do this without too much effort is by using C++23's std::generator:

    template<typename T> std::generator<const T&> get_line_view(std::istream& input) { … std::istream_iterator<T> begin(line_stream), end(); for (auto it = begin; it != end; ++it) { co_yield *it; } } 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.