Reading a file in c++

tironci · 11-03-2009, 09:25 PM

hi, i am new to c++.i am trying to read from a file and put the information into an array and then display the contents of the array to the screen. i do not get any error when i run this project from eclipse and when i run it from Microsoft Visual Studio it says that the riders.exe has stopped working. can someone please take a look and tell me why its not working.

.h file

Code:

#include <string>


using namespace std;

class Motorcycles{

	private:

	string m_name;
	string m_brand;
	string m_measure;
	float m_engine;
	float m_price;

public:

	Motorcycles();
	void Motorcylces(string brand, string name,float engine, string measure,  float price);
	void toString(void);

};

.cpp file

Code:

#include <iostream>
#include <stdio.h>
#include <string>
#include <fstream>
#include <sstream>
#include "easyriders.h"

using namespace std;

Motorcycles::Motorcycles(){

	m_brand = "Empty";
	m_name = "Empty";
	m_engine = 0.0;
	m_measure = "Empty";
	m_price = 0.0;

}

 void Motorcycles::Motorcylces(string brand, string name, float engine, string measure,  float price){

	m_brand = brand;
	m_name = name;
	m_engine = engine;
	m_measure = measure;
	m_price = price;

}

void Motorcycles::toString(void){

	cout << m_brand << " " << m_name << " " << m_engine << " " << m_measure << " " << m_price << endl;

}


Motorcycles getObj(string s){

	string brand, name, measure;
	float engine, price;

	istringstream iss(s);

	iss >> brand >> name >> engine >> measure >> price;

	Motorcycles bike;

	bike.Motorcylces(brand,name,engine,measure,price);

	return bike;

}


int main(){

	string binfo;


	ifstream bikes;
	bikes.open("bikes.txt");
	Motorcycles array[10];


	if(bikes.is_open()){

		for ( int i = 0; getline(bikes,binfo); i++){

			array[i] = getObj(binfo);
		}
	}
	for (int x = 0; x < 10 ; x++){

		array[x].toString();

	}

return 0;

}

nadroj · 11-03-2009, 10:17 PM

Can you verify whether the file is being opened or not? Heres a part of your "main", with irrelevant details left out

Code:

	Motorcycles array[10];

	if(bikes.is_open()){
           // ...
	}
	for (int x = 0; x < 10 ; x++){
		array[x].toString();

	}

As you can see, the last block (the for loop) always gets executed. If the file failed to open, then the "array" array is not initialized, and youll get a segfault/null pointer exception on the coloured line. This for loop should probably be inside the if block, so you only print the elements if the file was opened and read successfully.

Also, aside, I would name the class "Motorcycle"--singular--opposed to "Motorcycles", because the latter is misleading.

johnsfine · 11-04-2009, 08:25 AM

Quote:

Originally Posted by tironci

when i run it from Microsoft Visual Studio it says that the riders.exe has stopped working.

It didn't say exactly that did it?

I think you are incorrectly paraphrasing a message that you misunderstood.

I suspect that your problem is in misuse of Visual Studio rather than in your program itself.

Visual Studio can be very confusing to beginners.

If you are a beginner at Visual Studio, tell us more details about version of Visual Studio and about what specific actions you took to build your program and to run it and to get a look at the output from running it (which can be surprisingly hard in Visual Studio).

Of course if you are not a beginner at Visual Studio, I probably guessed wrong and also missed some bug in your source code. So if you are confident in your ability to operate Visual Studio, tell us that.

tironci · 11-04-2009, 08:52 AM

this is not my first time with visual studio, i think i can create projects the right way. i added a line in the main()'s if loop. the line executes but it never goes to the for loop. i have attached a screen shot of what i get when i run it.

Code:

if(bikes.is_open()){

		cout << "Cannot open file"<<endl;


		for ( int i = 0; getline(bikes,binfo); i++){

			//istringstream iss (binfo);

			array[i] = getObj(binfo);

			//cout << array[i] << endl;

		}

	
		for (int x = 0; x < 10 ; x++){

			array[x].toString();

		}
	}

johnsfine · 11-04-2009, 09:23 AM

You put the "Cannot open file" message in the path where is has opened the file.

The message I didn't believe is from Windows, not from Visual Studio. I just use XP, not the newer stupider versions of Windows, so I didn't recognize that message.

Anyway, it seems there is a bug in your source code.

Earlier I didn't notice your loop depends on the contents of the file and is not limited to the ten items for which you have room in the array. So if the loop went more than ten times, that would exactly fit the symptoms.

Maybe you accidentally got a blank line at the end of bikes.txt, maybe in converting to a Microsoft environment.

The way you coded your program, an extra line at the end of bikes.txt would cause the crash.

Also (but not related to the crash) it is very bad style to have a function that is a slight misspelling of the class name

Code:

void Motorcycles::Motorcylces

The way you used that, it would have been better to have a non default constructor:

Code:

Motorcycles::Motorcycles(string brand, string name, float engine, string measure,  float price) :
    m_name ( name ),
    m_brand ( brand ),
    m_measure ( measure ),
    m_engine ( engine ),
    m_price ( price )
    {}

Then instead of

Code:

	Motorcycles bike;
	bike.Motorcylces(brand,name,engine,measure,price);

use

Code:

	Motorcycles bike(brand,name,engine,measure,price);

But if you really want an init function instead of a constructor, it is much better to give it a name such as init, rather than misspell the constructor name.

tironci · 11-04-2009, 10:06 AM

my bad, i didn't realize i misspelled the names. that was not my intention. although i fixed all the errors and changed to your suggestions i still get that Bikes.exe error.

nadroj · 11-04-2009, 10:36 AM

Code:

if(bikes.is_open()){

		cout << "Cannot open file"<<endl;

This message is obviously misleading--it was able to open the file! Anyway, the easiest thing to do is to put print statements (or use a debugger) throughout your code, and find out the last message that printed before an error occurred. Once you found this, you found the exact line the error occurs on. Then you can tell us the line it occurred on.

Code:

if(bikes.is_open()){

		cout << "here 1 "<<endl;


		for ( int i = 0; getline(bikes,binfo); i++){
     		        cout << "here 2 "<<endl;

			//istringstream iss (binfo);
			array[i] = getObj(binfo);
     		        cout << "here 3 "<<endl;     		        
			//cout << array[i] << endl;

		}

                cout << "here 3 "<<endl;	
		for (int x = 0; x < 10 ; x++){
                cout << "here 4 "<<endl;	
			array[x].toString();
                cout << "here 5 "<<endl;	

		}
                cout << "here 6 "<<endl;	
	}

johnsfine · 11-04-2009, 12:11 PM

Quote:

Originally Posted by tironci

my bad, i didn't realize i misspelled the names. that was not my intention.

But if you hadn't misspelled that name, the compiler should reject it, because the name is reserved for constructors.

But I said that was not the problem. That was just some bad code.

Quote:

i fixed all the errors

Apparently not.

How can we help further if you say that without explanation or new sample code.

I think the main error was that there were more than ten lines in bikes.txt. Did you find that? Did you fix that?

The program ought to protect itself so having more than ten lines in the input file wouldn't make it crash. Did you fix that? How?

Quote:

changed to your suggestions i still get that Bikes.exe error.

For the important error I didn't maker any suggestion (until this post). I just gave my estimate of what the error was.

Changing the constructor to my suggestion is nice, but it wouldn't fix the main error.

smeezekitty · 11-04-2009, 12:18 PM

Quote:

Originally Posted by johnsfine

It didn't say exactly that did it?

I think you are incorrectly paraphrasing a message that you misunderstood.

I suspect that your problem is in misuse of Visual Studio rather than in your program itself.

Visual Studio can be very confusing to beginners.

If you are a beginner at Visual Studio, tell us more details about version of Visual Studio and about what specific actions you took to build your program and to run it and to get a look at the output from running it (which can be surprisingly hard in Visual Studio).

Of course if you are not a beginner at Visual Studio, I probably guessed wrong and also missed some bug in your source code. So if you are confident in your ability to operate Visual Studio, tell us that.

No, its a runtime not a compile time error.
Ether a memory viloation or illegal instruction.

tironci · 11-04-2009, 12:30 PM

apart from my misspelling there was nothing wrong with the code. the professor had put tabs and extra spaces in the file, because he wanted us to learn the hard way that istringstream doesnt like tabs and extra line breaks.
this is the code that is working.

Code:

#include "stdafx.h"
#include <iostream>
#include <stdio.h>
#include <string>
#include <fstream>
#include <sstream>
#include "bikes.h"

using namespace std;

Motorcycles::Motorcycles(){

	m_brand = "Empty";
	m_name = "Empty";
	m_engine = 0.0;
	m_measure = "Empty";
	m_price = 0.0;

}

Motorcycles::Motorcycles(string brand, string name, float engine, string measure,  float price){

m_brand = brand;
m_name = name;
m_engine = engine;
m_measure = measure;
m_price = price;

}

void Motorcycles::toString(void){

	cout << m_brand << " " << m_name << " " << m_engine << " " << m_measure << " " << m_price << endl;

}


Motorcycles getObj(string s){

	string brand, name, measure;
	float engine, price;

	istringstream iss(s);

	iss >> brand >> name >> engine >> measure >> price;

	Motorcycles bike(brand,name,engine,measure,price);

	return bike;

}


int main(){

	string binfo;


	ifstream bikes;
	bikes.open("bikes.txt");
	Motorcycles array[10];


	if(bikes.is_open()){	

		for ( int i = 0; getline(bikes,binfo); i++){		

			array[i] = getObj(binfo);		
		}
		for (int x = 0; x < 7 ; x++){
			
			array[x].toString();	
		}
	}
	bikes.close();

	return 0;

}

thank you to all the people that responded to my post.

johnsfine · 11-04-2009, 12:39 PM

Quote:

Originally Posted by smeezekitty

No, its a runtime not a compile time error.

We are way past that. I didn't believe a "Visual Studio" message was quoted correctly. It turns out it was quoted correctly but wasn't a Visual Studio message (it was a Windows message). Simple misunderstanding, now understood, no longer a problem.

My current working theory is that the failure is due to the line

Code:

array[i] = getObj(binfo);

executed when i is 10, because i is only allocated for 0..9.

We are waiting for tironci to confirm or contradict that theory and/or follow nadroj's advice to add so many printf's that the output could confirm "here 2" happens more than ten times, or if that is incorrect, would likely nail down the real bug.

If you have another theory about the real bug or other constructive suggestion, that would be interesting.

But meanwhile, a post such as you just made (excess quoting and useless extra comment) just serves to make it harder to find the constructive posts. I'm sorry this reply is impolite. I'd prefer not to be. But I think it needed to be said.

johnsfine · 11-04-2009, 12:48 PM

Quote:

Originally Posted by tironci

apart from my misspelling there was nothing wrong with the code. the professor had put tabs and extra spaces in the file, because he wanted us to learn the hard way that istringstream doesnt like tabs and extra line breaks.
this is the code that is working.

I'm not your instructor, so I'm only guessing, but maybe he also wanted you to learn some defensive programming.

Is code that crashes when the input is bad "wrong" or is the input the only thing that is "wrong" in that case?

That is a matter of opinion and a matter of degree.

Sometimes it just isn't practical to defend against all possible bad input. So I would not accept the absolute rule (which I have frequently seen) that the program is "wrong" if there is any input that makes it crash.

But almost any experienced programmer would say your program is "wrong", because crashing when the input file has more than ten lines is too severe for a simple input error and too easy to fix with a minor change to your source code.