Trying to split a text file into multiple text files in C++

Question

I am trying to get this code to split an input file into two files. I want the code to split so that one new file contains all the odd characters and the other file contains all the even characters. My code gives me no errors and it produces two new files but the two new files contain nothing in them. I am wondering what is wrong with my code(I am sure there is a lot wrong with it). I am still pretty new to programming.

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

void split(char sourceFile[], char destFile1[], char destFile2[]) {
    int chars = 0;
    ifstream sFile;
    sFile.open(sourceFile);
    ofstream file1;
    file1.open(destFile1);
    ofstream file2;
    file2.open(destFile2);

    while (!sFile.eof()) {
        sFile.read(sourceFile + chars, 1);

        cout << sourceFile[chars];
        if (chars % 2 == 0) {
            file1 << sourceFile[chars];
        } else {
            file2 << sourceFile[chars];
        }
        chars++;


    }
}

int main() {
    split("text.txt", "one.txt", "two.txt");
    return 0;
}

Answer 1

Not an answer to the question but a somewhat compact version of a program implementing the desired goal:

#include <algorithm>
#include <iterator>
#include <fstream>
#include <functional>

int main()
{
    typedef std::ostreambuf_iterator<char> oit;
    unsigned int chars(0);
    std::for_each(std::istreambuf_iterator<char>(
                      std::ifstream("text.txt") >> std::noskipws),
                  std::istreambuf_iterator<char>(),
                  std::bind([=](oit _1, oit _2, char c) mutable {
                          *(chars++ % 2? _1: _2)++ = c; },
                      oit(std::ofstream("one.txt") << std::dec),
                      oit(std::ofstream("two.txt") << std::dec),
                      std::placeholders::_1));
}

Answer 2

Some very serious problems:

• You're loop control is meaningless. It's not well defined when 'istream::eof()' becomes true, but when reading byte by byte, as you are doing, it will probably cause you to enter the loop one more time than you want.

• A related issue is that you don't verify that the read succeeded. The success of the read should be your loop control.

• You're reading into a character literal, which is undefined behavior (and will crash on a lot of systems); after enough reads, you're reading beyond the end of the literal, into undefined memory. You really should use a local buffer for the read.

• You don't do any error checking. At the very least, you should verify that you've successfully opened the files, and you should close the output files, and check their status after the close, to be sure that the writes have worked.

I'd probably use something along the lines:

void
split( std::string const& source, 
       std::string const& dest1,
       std::string const& dest2 )
{
    std::istream in( source.c_str() );
    if ( ! in.is_open() ) {
        //  Cannot open source...
    }
    std::ostream out1( source.c_str() );
    if ( ! out1.is_open() ) {
        //  Cannot create dest1
    }
    std::ostream out2( source.c_str() );
    if ( ! out2.is_open() ) {
        //  Cannot create dest2
    }
    std::ostream* currentOut = &out1;
    std::ostream* otherOut = &out2;
    char ch;
    while ( in.get( ch ) ) {
        currentOut->put( ch );
        std::swap( currentOut, otherOut );
    }
    out1.close();
    if ( !out1 ) {
        //  Write error on dest1...
    }
    out2.close();
    if ( !out2 ) {
        //  Write error on dest2...
    }
}

(As you can see, the actual loop which does the copying is extremely simple. Most of the code is involved in error handling.)

Answer 3

There's a lot wrong with it (as you said)

1) All your files are ifstream, if you want to output to a file use ofstream.

2) You should not attempt to use the file names as variables for your code. Just declare a new char variable.

3) Use get not read to read single characters.

4) Test for end of file correctly.

5) Don't close the files inside the loop, in fact don't close the files at all, it will happen automatically when you exit the function.

6) What on earth is chars%2 == 5 about? It will never be true.

Putting that together

char ch;
while (sFile.get(ch))
{
    if (chars % 2 == 0)
        file1 << ch;
    else
        file2 << ch;
    chars++;
}

Answer 4

You forgot a ';' and misspelled 'sourcFile'

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

void split(char sourceFile[], char destFile1[], char destFile2[])
{
int chars = 0;

ifstream sFile;
sFile.open (sourceFile);

ofstream file1;
file1.open (destFile1);

ofstream file2;
file2.open (destFile2);

while (!sFile.eof())
{
    sFile.read(sourceFile+chars,1); // forgot ';'

    cout << sourceFile[chars];
    if (chars % 2 == 0)
    {
        file1<< sourceFile[chars];
    }
    else if(chars % 2 == 5)
    {
        file2 << sourceFile[chars]; // misspelled 'sourcFile'
    }
    chars++;

sFile.close();
file1.close();
file2.close();
}

}

int main()
{
split("text.txt", "one.txt", "two.txt");
return 0;
}

Haven't tested the output though. If it doesn't work, could you give us the inputfiles to try for ourselves?

Answer 5

Your indentation is misleading! The file close() calls are inside the loop.

And: i % 2 == 5 is never true. How about if (i % 2 == 0) { ... } else { ... } ?