0

I have a class with an inner class which has a member ifstream. When executing the code throws an AccessViolationException in TestDataHelper::getNextLine() at if(!datafile->eof()).

Debugging showed me that the constructor is working fine. The file is created and the first line is read. But when I check the datafile after the assignment is done (after TestDataHelper helper = TestDataHelper("data.csv");), the datafile becomes an invalid pointer.

The code:

IfstreamTester.h

#include <vector>
#include <iostream>
#include <string>
#include <sstream>

class IfstreamTester
{
public:
    IfstreamTester(void);
    ~IfstreamTester(void);

    void doStuff();

private:
    std::ifstream file;

    class TestDataHelper{
    private:
        std::ifstream *datafile;
    public:
        TestDataHelper(std::string filename);
        ~TestDataHelper();
        std::vector<double>* getNextLine();
    };
};

IfstreamTester.cpp

#include "IfstreamTester.h"

using namespace std;

IfstreamTester::IfstreamTester(void)
{}

IfstreamTester::~IfstreamTester(void)
{}

void IfstreamTester::doStuff()
{
    TestDataHelper helper = TestDataHelper("data.csv");
    // for every line in the file
    vector<double>* v;
    do
    {
        v = helper.getNextLine();
        cout << v << endl;
        delete v;
    } while(v != NULL);
}

IfstreamTester::TestDataHelper::TestDataHelper(std::string filename)
{
    datafile = new ifstream(filename.c_str());
    // read the header line
    string line;
    getline(*datafile, line);
}

vector<double>* IfstreamTester::TestDataHelper::getNextLine()
{
    if(!datafile->eof())
    {
        // readline
        string line = "";
        getline(*datafile, line);
        string delimiter = ",";
        vector<double>* res = new vector<double>();
        size_t pos = 0;
        // and tokenize line into vector of doubles
        while ((pos = line.find(delimiter)) != std::string::npos)
        {
            stringstream conv;            
            double token;
            conv << line.substr(0, pos);
            conv >> token;
            res->push_back(token);
            line.erase(0, pos + delimiter.length());
        }
        return res;
    }
    else
    {
        return NULL;
    }
}

IfstreamTester::TestDataHelper::~TestDataHelper()
{
    datafile->close();
    delete datafile;
}

and the main:

#include "IfstreamTester.h"

void main()
{
    IfstreamTester s = IfstreamTester();
    s.doStuff();
}

Anyone an idea why this is so?

EDIT: I am using Microsoft Visual Studio 2008 Express

Mike de Dood
  • 393
  • 1
  • 10

3 Answers3

1

while(v != NULL) looks fairly wrong. Also did you mean to write your loop as follows?

vector<double>* v = NULL;
do
{
    v = helper.getNextLine();
    cout << v << endl;
    delete v;
} while(v != NULL);
πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • getNextLine is made such that it returns a NULL pointer when the end is reached. But anyway, this is not remotely explaining nor solving the problem of the ifstream object becoming invalid after the assigmnent constructor call. – Mike de Dood Mar 05 '14 at 11:03
  • @MikedeDood At least this was to long for a comment. As for your problems with `ifstream*` becoming invalid, I'd recommend using a `std::unique_ptr` instead of a raw pointer. You're closing and deleting the `ifstream*` in your `TestDataHelper` destructor, this is a violation of the [rule-of-three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three/4172724#4172724) principles. – πάντα ῥεῖ Mar 05 '14 at 11:38
  • I cannot use std::unique_ptr (it is c++98 and boost may not be used). – Mike de Dood Mar 05 '14 at 12:20
  • @MikedeDood Why aren't you using a plain instance of `ifstream` (not to be copied), and the filename as a separate member? You can use the `ifstream::open()` separately then. You don't need to call `close()` on destruction BTW. – πάντα ῥεῖ Mar 05 '14 at 12:24
  • @πάνταῥεῖ I want to have an object that on construction opens the file and with every call of getNextLine reads the next line in the file and parses it into a vector of doubles. I don't see how that could work without having the file as class member. – Mike de Dood Mar 05 '14 at 12:30
  • @MikedeDood You **can** use `ifstream::open()` in the constructor of your helper class. _'work without having the file as class member'_ I didn't say you should have no member, but instead of a `ifstream*` have a plain `ifstream` instance as member. – πάντα ῥεῖ Mar 05 '14 at 13:22
1

Your compiler isn't doing you any favors. It should be puking, telling you std::ifstream isn't copyable and the default copy-constructor of its containing parent , IfstreamTester, should therefore also be implicitly deleted.

I don't know what toolchain you're using, but clang++ 3.3 and g++ 4.7 both puke on this. Your compiler is apparently allowing it through because of a copy-elision assumption, then failing to actually elide the copy in debug. Either add a copy-constructor that doesn't member copy the std::ifstream member, or declare main() as:

int main()
{
    IfstreamTester s;
    s.doStuff();
}

The non-standard void main() declaration in your code has me (likely incorrectly) deducing you're using either an MS or Borland toolchain. Whoever is allowing this (both the odd main() and the non-copyable std::ifstream member to be copied), isn't playing by the rules.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Making the copy constructor, the assignment operator private and adapting the code respectively solved my problem. Thanks – Mike de Dood Mar 05 '14 at 12:36
0

I tried do compile this using g++ 4.6.3:

I had to add #include <fstream> and change IfstreamTester s = IfstreamTester(); to IfstreamTester s;

Daniele
  • 821
  • 7
  • 18