HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Writing a vector to a file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filewritingvector

Problem

The following function (function template rather) is to take a vector of any type and write it a file using a filestream object.

template
void Vec_WriteFile(const vector& vec, ifstream file, char del = '\n')
{
   for (const T& element : vec) file << element << del;
   file << "\b ";
}


This function is very straightforward and simple; however, I'm a student to C++ and would love to learn different ways of doing even the simplest of tasks.

Solution

You need to include the headers your code depends on; don't assume that the user of your code will include them for you:

#include 
#include   // see note below


Never require your user to bring names into the current namespace; type names used in your public interface should always be qualified:

template
void Vec_WriteFile(const std::vector& vec, std::ifstream file, char del = '\n')


Don't attempt to write to an istream. Your output should go to an ostream instead. You could use an ofstream, but that's quite specific. Instead, you should accept a reference to any ostream; the user can then pass an ofstream or std::cout or any other output stream:

template
void Vec_WriteFile(const std::vector& vec, std::ostream& file, char del = '\n')


There's nothing in your code that wouldn't work for a std::list equally as well as for a std::vector, so you can be more general:

template
void Vec_WriteFile(const Container& vec, std::ostream& file, char del = '\n')


Naming: del evokes "delete" in my head before I reset and think "delimiter". It might be better written in full, or perhaps as sep for "separator".

Layout: at first glance, it's not obvious which code is controlled by the for condition, as you have inconsistent indentation.

Writing a backspace over the final delimiter isn't the same as not writing the delimiter. If you want the delimiter between items but not after the final item, the usual way is to track whether the loop is in its first iteration:

bool first = true;
for (...) {
    if (first)
        first = false;
    else
        file << del;
    file << element;
}


Error reporting: instead of just assuming that the writes are all successful, we could report via the return value from the function. This may help the user remember to deal with errors, which could be overlooked if they have to check the stream state after the call.

My version:

#include 

template
std::ostream& write_container(const Container& c,
                     std::ostream& out,
                     char delimiter = '\n')
{
    bool write_sep = false;
    for (const auto& e: c) {
        if (write_sep)
            out << delimiter;
        else
            write_sep = true;
        out << e;
    }
    return out;
}

Code Snippets

#include <vector>
#include <ostream>  // see note below
template<class T>
void Vec_WriteFile(const std::vector<T>& vec, std::ifstream file, char del = '\n')
template<class T>
void Vec_WriteFile(const std::vector<T>& vec, std::ostream& file, char del = '\n')
template<class Container>
void Vec_WriteFile(const Container& vec, std::ostream& file, char del = '\n')
bool first = true;
for (...) {
    if (first)
        first = false;
    else
        file << del;
    file << element;
}

Context

StackExchange Code Review Q#157005, answer score: 5

Revisions (0)

No revisions yet.