Error with I/O function

[BarelyOtaku]

Hey guys I'm getting an error of

"Terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_S_construct null not valid

All I did was

/*
 * reader.h
 *
 *  Created on: Jul 31, 2015
 *     
 */

#ifndef READER_H
#define READER_H

#include "data.h"
#include <fstream>
#include <string>


class Reader{

  public:
	void readAndWrite(std::ifstream &read) {
		std::cout << "Test: lines";
		std::cout << getLines(read); //counts the number of lines
		Data D(getLines(read)); //I'm actually confused on how to apply templates. 
//I have three cases of either int, float, or char. 
//How do I apply a general case template class for it to generate depending on the context of what it reads?
		for(int i=0 ; i < getLines(read); i++){
			    read >> dType;
				read >> dSize;
				std::getline(read, dSeq);
				D.set(dType, dSize, dSeq);
		}
		std::cout << D.getSize();
	}

	int getLines(std::ifstream &read){ 
		int numberoflines = 0;
		std::string temp;
		while (std::getline(read, temp))
			numberoflines++;
		return numberoflines;
	}
  private:
	int numberoflines;
	std::string dType;
	int dSize;
	std::string dSeq;
	std::string dummy;

};

#endif

In my driver I did something like this

/*
 * main.cpp
 *
 *  Created on: Jul 31, 2015
 *      
 */

#include "reader.h"
#include <iostream>
#include <fstream>
#include <string>

int main(int argc, char *argv[]){
	std::cout << "Have " << argc << " arguments:" << std::endl;
	   for (int i = 0; i < argc; ++i) {
	       std::cout << argv[i] << "\n";
	   }
	 std::ifstream inFile(argv[1]);
	Reader obj;
	obj.readAndWrite(inFile);


}

I set a second argument path to my text file and the output of all this looks like

"Terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_S_construct null not valid 
Have 2 arguments:
/home/jacky/Dropbox/eclipse-workspace/assignment-4f/Debug/assignment-4f
/home/jacky/Dropbox/eclipse-workspace/assignment-4f/src/testio.dat
Test: lines1

The error just appears anywhere since it uses the CERR stream so I have no idea where the problem is.

Edit: Added my data header

/*
 * data.h
 *
 *  Created on: Jul 30, 2015
 *      
 */

#ifndef DATA_H_
#define DATA_H_

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


class Data{
public:
	Data(int number) :
		dType(0),
		dSize(0),
		numberOfLines (number)
{
		arrayPtr = new int[dSize];
}

	void set(std::string type,int size, std::string seq){
		dType = type;
		dSize = size;
		dSeq = seq;
	}

	void outputReversedString(){
		std::stringstream tempStream(dSeq);
		std::string tempString;
		for (int i = 0; i < dSize; i++){
			tempStream >> arrayPtr[i];
		}
		for(int i = dSize-1; i >= 0; i++){
			std::cout << arrayPtr[i];
		}

	}
	std::string getType() const{
		return dType;
	}

	int getSize() const{
		return dSize;
	}


private:
	std::string dSeq;
	std::string dType;
	int dSize;
	int numberOfLines;
	int *arrayPtr;
};

#endif /* DATA_H_ */

Please save me

[TheIdeasMan]

class Data{
public:
	Data(int number) :
		dType(0),
		dSize(0),
		numberOfLines (number)
{
		arrayPtr = new int[dSize];  // dSize is 0, did you mean number ?
}

Also, could you have 1 ctor for the Data class, that sets all the member variables? That way you can minimise the risk of uninitialised variables.

You appear to have a memory leak, new without delete . Avoid new & delete whenever you can - investigate smart pointers instead.

Prefer to use std::vector rather than arrays.

How do things look when you use a debugger? This will find all kinds of things quickly.

Hope this helps :+)

[TheIdeasMan]

Hi,

With regard to your question about templates:

BarelyOtaku wrote:

//I have three cases of either int, float, or char. 
//How do I apply a general case template class for it to generate depending on the context of what it reads?
 

If you want to do different things depending on the underlying type, then you need template specialisation:

http://en.cppreference.com/w/cpp/language/template_specialization

So the idea would be to write a different specialisation for each type that you want. Hopefully the code is not too similar.

On the other hand if you just want to restrict the types that can be used with the template, you can use static_assert and type traits, although this is only a compile time checking facility:

http://en.cppreference.com/w/cpp/language/static_assert

This might be better, you might have one set of code for all the types.

An example you could adapt, Cubbi helped me with this awhile back:

http://www.cplusplus.com/forum/lounge/111518/#msg608851

You could use the type traits in conjunction with exceptions, to handle run time problems - for example in the input there is a std::string with 1 char in it, instead of a char . In this case you could just cast it to a char anyway, but if it has multiple chars, throw the exception.

Another way might be to write the specialisation for std::string that throws, not sure on that though, I haven't tried it.

BTW, you should avoid writing cpp code that does something, into a header - that can become pure evil very quickly . <8+| Separate the code so declarations go in a .hpp file (a cpp header) , and function definitions into a .cpp file. Name the files exactly the same as the class name.

Remember though, templates have to go in their entirety code for functions and all into a header file.

Hope all goes well

[BarelyOtaku]

Thank you so much TheIdeasMan it ended up being the way I initialize my variable!

Also now I have another problem, my read function isn't getting what I want it to do.
type(int, char, or float) size(of following) number sequence

So it would look like:

int 5 12345
float 1 1.40
char 4 a b c d

However after I set the dSize and dType, my output of dSize ends up being 1.

My text file however has :
test 123 123

So technically my output should be 123, am I writing my input stream wrong?

[TheIdeasMan]

		for(int i=0 ; i < getLines(read); i++){
			    read >> dType;
				read >> dSize;
				std::getline(read, dSeq);
				D.set(dType, dSize, dSeq);
		}

I did some in-brain debugging with watchlist :+D -> the for loop overwrites the values in D each time on line 29, does that cause a problem?

Another thing with functions, they should only do one thing, so IMO read data, process data, & write data should be at least 3 functions. Store the data into a container of some kind, so it can be processed by the ProcessData function. Make use of a union, seen as the types are known and there is only 1 of them at a time - you could have a std::vector of the union type. A union is similar to a class or struct - they can have constructors and everything - but there are restrictions as it can only hold 1 type of data at a time.

http://en.cppreference.com/w/cpp/language/union

Seen as your input file has the actual type in it, that is way easier - never mind about all that template stuff.

[BarelyOtaku]

I reworked the loop into a different one, for some reason reading it through an object and reading it directly from main yields different results.

e.g.

/*
 * main.cpp
 *
 *  Created on: Jul 31, 2015
 *      Author: jacky
 */

#include "reader.h"
#include <iostream>
#include <fstream>
#include <string>

int main(int argc, char *argv[]){
	std::cout << "Have " << argc << " arguments:" << std::endl;
	   for (int i = 0; i < argc; ++i) {
	       std::cout << argv[i] << "\n";
	   }
	   
	   //Test for my loop
	 std::ifstream inFile(argv[1]);
	 std::string check;
	 int check1;
	 int check2;
	 while (inFile >> check){
		 inFile >> check1;
		 inFile >> check2;
	 }
	 std::cout << check << check1 << check2;

	 //Testing using the function
	Reader obj;
	obj.readAndWrite(inFile);

}

I run the function twice once, through the main and once through an object

/*
 * reader.h
 *
 *  Created on: Jul 31, 2015
 *      Author: jacky
 */

#ifndef READER_H
#define READER_H

#include "data.h"
#include <fstream>
#include <string>


class Reader{

  public:
	void readAndWrite(std::ifstream &read) {
		std::cout << "Test: lines";
		 //counts the number of lines
		std::cout << getLines(read);
		read.seekg(0);
		Data D(1); //I'm actually confused on how to apply templates.
		while (	read >> dType){
			read >> dSize;
		}
		std::cout << dType;
		D.set(dType, dSize);
		//std::cout << dSize;
				std::cout << D.getSize(); //Bare with this messy code 
//I'm trying to debug the source of the problem

	}

	int getLines(std::ifstream &read){
		int numberoflines = 0;
		std::string temp;
		while (std::getline(read, temp))
			numberoflines++;
		return numberoflines;
	}
  private:
	int numberoflines;
	std::string dType;
	int dSize;
	std::string dSeq;
	std::string dummy;

};

#endif 

My exact output is this

Have 2 arguments:
/home/jacky/Dropbox/eclipse-workspace/assignment-4f/Debug/assignment-4f
/home/jacky/Dropbox/eclipse-workspace/assignment-4f/src/testio.dat
test123123Test: lines01

And I have no idea how to use the Debugger so I can't give feedback about it D:

[TheIdeasMan]

And I have no idea how to use the Debugger so I can't give feedback about it D:

Ah .... What sort of IDE are you using? You are using Eclipse - I have that on my system too. There should be a Debug Menu, with start code in debug mode menu option, somewhere to set-up a watch-list, ways of putting break points in your code, ways to step through code 1 line at a time, step into or over functions.

Seriously this is a super worthwhile thing to learn - it will save you days of heartache and head scratching.

Ok, I found it -> Window Menu -> Open Perspective -> Debug It should have all of the stuff I mentioned above.

once you have set up your variables watchlist, breakpoints by double clicking to the left of the line of code where you want it, you can choose Run -> Debug There are Tool bar buttons that step 1 line at a time, but the keyboard short cuts are quicker.

Once you have this sorted, I think you should have a good go at the other things I mentioned.

Good Luck !!

[TheIdeasMan]

Hi,

Hopefully you have enough to go on with - I need to go back to bed again - it's now early morning at this end

Regards ZZZZZZZZzzzzzzzzzzzz............

[BarelyOtaku]

Alright thanks for your help TheIdeasMan!
I'll try my best on understanding this debug perspective.

[ne555]

> for some reason reading it through an object and reading it directly from main yields different results.

	 while (inFile >> check){ //while you can read
		 inFile >> check1;
		 inFile >> check2;
	 }
	//so here you can't read anymore
	 std::cout << check << check1 << check2;

	 //Testing using the function
	Reader obj;
	obj.readAndWrite(inFile); //this will attempt to read 

You make the same mistake in `readAndWrite()', your `getLines()' function already invalidated the stream.

[BarelyOtaku]

Thanks a lot ne I fixed it!
I added:

inFile.clear();
inFile.seekg(0);

and reset the status of my file.

[TheIdeasMan]

Hi,

If you had a std::vector of struct with a union inside - you could avoid the getlines function (just determines number of lines in file) altogether, and simplify the code in other ways:

struct FileRecord {
   std::string TypeIndicator;  // "int" or "float" or "char" as string literal
   std::size_t Size; // number of digits for int, number of floats, number of chars
   
   union Type {
      int Int_t;
      float Float_t[Size];
      char Char_t[Size];
   }
   // need a constructor
}

std::vector<FileRecord> FileData;

// gather input info - function
  // getline from file
  // populate FileRecord using a switch with stringstream
  // push_back to vector
// Process info if any - function
// Output info from vector  - function 

I haven't tested any of this - just putting the concept out there

http://en.cppreference.com/w/cpp/language/union

[BarelyOtaku]

Right now I still went ahead and used template xD. It's working so far though but only because I haven't tested the use of multiple lines. My teacher told me I didn't grasp the concept of templates and that I don't need to bother trying. So now I'm taking on the challenge to prove her wrong.

However, I'll definitely check out union next time

[TheIdeasMan]

Ok,

Are you going to make some of the alterations I mentioned? You might get better marks if you do.

I know it's tough - one struggles to write code, then struggle more to get it working, and once it finally does work - there is this bloke who wants you to reorganise everything :+) , and you have other assignments to do as well.

But there are somethings that are definitely worth doing - splitting the headers into .hpp and .cpp, break up the ReadAndWrite function, use std::vector instead of pointers and arrays (remove the need for getlines function), use smart pointers instead of allocating on the stack or using new .

My last post shows a way of achieving the goal without templates.

There are some downsides to using templates - granted it looks as though here you are only reading, storing & printing the data - not actually doing anything to the data. So if you do a template specialisation for those 3 types, the compiler is going to generate a class for each type. The template code you write for that will be almost identical for each specialisation, and you will need a container of pointers to store them. All this just to store the info.

By using the struct with the union along with a vector to store them, things will be much easier & efficient IMO. Maybe this is why your prof said there was no need to use templates. On the other hand it is good to have a go at using them.

Also, I am always prepared for experts like ne555 to tear to shreds what I have said - if it is wrong :+)

[BarelyOtaku]

Yeah I split the header and cpp,
the only reason I had them clustered up was because I was experimenting with templates and it was easier if I kept them in one header file.

I also reimplemented the function readAndWrite() by breaking it into three functions and then grouping them up under the readAndWrite() function (which is now called processData()).

I'm really hesitant on using struct, union and vector because our teacher didn't cover it in class. On top of that she didn't allow us to use vectors on our last assignments. We had to use specifically pointers and built-in arrays.

Whatever I'll try it I just hope she doesn't accuse me of plagiarism XD

Could I just have some pointers because I have never seen any of these concepts.
For example:
- Do I declare struct in a data.h along with other functions? Or would I need a new file?
- You mentioned I need a constructor so would I basically implement and initialize a struct the same way I do with a class?
- I need to use the try and catch method, would I implement this in the main or struct file?

[TheIdeasMan]

Whatever I'll try it I just hope she doesn't accuse me of plagiarism XD

I reckon the best policy is complete honesty - tell your prof that you are getting advice from this site, make sure beforehand that it is OK to use vector & union etc. No point in doing all that work - the failing the assignment. I always try to present Ideas & Concepts rather than write code - my Tag is TheIdeasMan after all :+)

- Do I declare struct in a data.h along with other functions? Or would I need a new file?

Declarations go into a header file - I prefer .hpp , then you just #include that where ever you need .

- You mentioned I need a constructor so would I basically implement and initialize a struct the same way I do with a class?

Yes. Have a read of the documentation though. A struct is the same as a class in every respect, except the default access is public: It's handy when the struct will be a type stored in a container, so one doesn't have to bother with accessor functions for the struct.

- I need to use the try and catch method, would I implement this in the main or struct file?

Crikey - do you think exceptions are worth it for this??

So the purpose of exceptions is throw whenever a class invariant is violated. An invariant is a value of a member variable that must make sense for the class to operate correctly. For example a someone's age must be positive.

To use exceptions one needs to try anything that is going to change a member value. So this includes upon (technically before) construction. Use function try block in the initialiser list to catch broad errors like when the input is negative when unsigned is expected. Create your own exception types to test valid ranges say for Longitude is 0.0 to 180.0 . This way all the member values are validated before the object is created, and creation doesn't happen at all if any of them throw. The try catch part is in the code that creates a new object.

Obviously we need to throw whenever a member value might change and be invalid during runtime - so this will be in the member functions, the try and catch part will be where the function are called from.

[BarelyOtaku]

Nah there are some things that I don't approve but it's required in the assignment so I can't argue with it.
For example, the assignment specified "a" function to read, reverse and write the sequence.
So I guess having multiple functions that make up the read, reverse and write sequence function is a valid work around.

[TheIdeasMan]

the assignment specified "a" function to read, reverse and write the sequence

Well I would consider the concept that a function does 1 thing, much more important than a slightly misleading assignment instruction. I wouldn't take it that literally - so I wouldn't have a function called ReadAndWrite in the class - even though it might call 3 other functions. I would just have the 3 member functions. However, ask your prof and see what she thinks of this interpretation, or whether it should really be taken literally.

The other way of looking at it is to have the 3 member functions, and then a function ReadReverseWriteData external to any class, in the scope of main() - it can call the objects functions.

[BarelyOtaku]

Yeah I don't know why I took it that way and forced myself to write only one function.

On a side note:

How would I write the read function if I want to push a structure of data into the vector.
I had the same problem with template class, how would I instantiate an object every time it reads while ensure it stays in the container.

e.g. for some function

#include "reader.h"
void Reader::readData(std::ifstream& read){
    Storage tempStorage;
    while (read >> Storage.dType){
        read >> Storage.dSize;
        read >> Storage.dSeq; // dSeq is my union Type here I think there's a problem
        storageVector.push(tempStorage);
    }
}

Would my object stay at the end of the function since it is created locally? Another problem I have is that the dSeq are numbers separated with a whitespace. Therefore, in order to process it into the Union array would I create a

 for(int i = 0; i < dSize; i++)
    read >> Storage.dSeq[i];

Finally I'm attempting to use the sample code you provided and for some reason

#ifndef STORAGE_H_
#define STORAGE_H_

#include <string>
struct Storage{

	std::string dType;
	int dSize;

	//Had to declare union after dSize else it wouldn't recognize.
	union Type {
			int arrayInt[dSize];
			float arrayFloat[dSize];
			char arrayChar[dSize];
		};

	Type dSeq;
	Storage();
	Storage(std::string type, int size, Type seq) :
		dType(type),
		dSize(size),
		dSeq(seq)
	{}
};



#endif /* STORAGE_H_ */ 

I get an error saying that invalid use of non-static data member Storage::dSize;
Then all my data members in my union Type gets errors saying "from this location". I'm guessing the problem is that dSize isn't initialized which causes an error with the arrays.

[TheIdeasMan]

Yeah I wasn't thinking straight with that code (although I did think about it while cooking dinner )- array size must be known at compile time. Rather than dynamically allocate an array - I thought maybe a vector of the union might be better. And declare the union separately & make sure it is constructed properly. So the FileRecord class will contain the Type, number of, and a vector of the union which are the actual values. The class will have the 3 functions mentioned earlier. Then on top of that there will be a vector of the FileRecords to store all the data in the file.

I also didn't name an object of the union type - this will be fixed up as it will be a member variable. I guess I sometimes get discombobulated when I just type code straight in here.

I will have a go at actually writing some code, not sure how long that might take - depends on what other distractions there are this evening. Also I am full of head cold & Sudafed at the moment, so hopefully the compiler will help prevent any freaky results. 8+D