7
\$\begingroup\$

I wrote (several months ago) a program that will take an input file, with a provided std::string, and append that to each individual line in the file. Looking at the code again, I am wondering about the performance of this program, possibly if the file is very large or any of the lines/input string is long.

#include <fstream>
#include <iostream>
#include <vector>

// argv[1] is input file name
// argv[2] is output file name
// argv[3] is string to add
int main(int argc, const char * argv[])
{
    if (argc != 4) {
        std::cout << "Needs input file and string to add" << std::endl;
        return EXIT_FAILURE;
    }
    std::ifstream in_file(argv[1]);
    std::string str;
    std::vector<std::string> vec, result_vec;
    while (std::getline(in_file, str)) {
        vec.push_back(str);
    }

    std::for_each(vec.begin(), vec.end(), [&](std::string str) {
        result_vec.push_back(str + argv[3]);
    });

    std::ofstream out_file(argv[2]);
    std::copy(result_vec.begin(), result_vec.end(), std::ostream_iterator<std::string>(out_file, "\n"));
}
\$\endgroup\$
6
\$\begingroup\$

There is no need to store all lines from the input file in memory. For each line read from the input you can append the string and write the result to the output file. This would save memory for large input files.

You should also check if opening the input and output file was successful, otherwise your program will silently do nothing if the files could not be opened/created.

std::ifstream in_file(argv[1]);
if (!in_file) {
    std::cerr << "Could not open input file\n";
    return EXIT_FAILURE;
}
std::ofstream out_file(argv[2]);
if (!out_file) {
    std::cerr << "Could not create output file\n";
    return EXIT_FAILURE;
}
const char * add_string = argv[3];

std::string str;
while (std::getline(in_file, str)) {
    out_file << str << add_string << "\n";
}
\$\endgroup\$
4
  • \$\begingroup\$ Curious: would doing this be slower for writing out to the file, since you're basically only incrementally writing to it instead of writing all at once? \$\endgroup\$ – Ryan Nov 25 '14 at 21:02
  • \$\begingroup\$ @Ryan There is only one solution to know for sure: time it :p \$\endgroup\$ – Morwenn Nov 25 '14 at 21:07
  • 1
    \$\begingroup\$ @Ryan: I don't think so. Both variants write sequentially to the file, and ofstream buffers the data in any case. \$\endgroup\$ – Martin R Nov 25 '14 at 21:08
  • \$\begingroup\$ Also you have removed all the cost of memory management. \$\endgroup\$ – Martin York Nov 25 '14 at 21:35
4
\$\begingroup\$

I like @Martin R's solution.
For this specific situation it is the best.

The code I present below is for learning purposes and shows a simple technique for converting input from one format to another. The technique is useful (but needs a a slightly more complex problem before it is worth using). But this problem provides a simple enough problem that understanding it becomes trivial.

#include <algorithm>
#include <iterator>
#include <utility>
#include <string>
#include <iostream>
#include <fstream>

// Define a structure for reading the input with.
// Including the input operator<<
struct Line
{
    std::string line;
    friend std::istream& operator>>(std::istream& str, Line& data)
    {
        return std::getline(str, data.line);
    }
};

// Define a structure that takes an
// input record and converts it to the format you want to use for output.
// Including the output operator>>
struct LineAppend
{
    static std::string  append;

    std::string line;
    LineAppend(Line const& inputLine)
        : line(inputLine.line)
    {}
    friend std::ostream& operator<<(std::ostream& str, LineAppend const& data)
    {
        return str << data.line << append << "\n";
    }
};

// Slightly hacky for this example    
std::string LineAppend::append;

The transformation then simply becomes a copy.

int main(int argc, char* argv[])
{
    std::ifstream   input(argv[1]);
    LineAppend::append   = argv[2];

    // Now copy the input to the output.
    std::copy(std::istream_iterator<Line>(input),
              std::istream_iterator<Line>(),
              std::ostream_iterator<LineAppend>(std::cout, "\n"));
}

Now moving to C++11 we can use the range based for:

// Define a class that can be used as range based container.
// This just means defining begin() and end() appropriately.
template<typename T>
class StreamReader
{
    std::istream&   str;
    public:
        StreamReader(std::istream& str)
            : str(str)
        {}

        std::istream_iterator<T>        begin()         {return std::istream_iterator<T>(str);}
        std::istream_iterator<T>        end()           {return std::istream_iterator<T>();}
        std::istream_iterator<T const>  begin() const   {return std::istream_iterator<T>(str);}
        std::istream_iterator<T const>  end() const     {return std::istream_iterator<T>();}
};

int main()
{
    std::ifstream   input(argv[1]);

     for(Line const& line : StreamReader<Line>(input))
    {
        std::cout << line.line << argv[2] << "\n";
    }
}
\$\endgroup\$
3
\$\begingroup\$
  • Print the error to std::cerr instead of std::cout. On the same line, add a "\n" to the end of the output instead of having a std::endl. However, this shouldn't affect performance since the program will terminate at that point. It's still a valid point in general, though.

  • I've never seen const used with argv, possibly because it's non-standard. I'm guessing your compiler is allowing this. This post has some information on this.

  • Any performance decrease may be due to all the push_back() calls done, which may become noticeable with larger files with many lines. I'm not too sure about any good alternatives, but it doesn't quite feel right to use a std::vector if it'll only have data added to it. After all, it's also meant for removing data from the back.

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