C string turning to junk when leaving a function

[MrBeavis]

Hey guys I am trying to create an open file function that checks the file type, I've done that but now when it leave the function it is turning to junk, Im using Cstring for now and not std::string as just good practice in case I ever run into a prestandard compiler that doesnt support string :)

here is the code

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

const int maxStringLength = 128;
const int maxCompanies = 200;
int i, j = 0;
const char line[maxStringLength] = {"*******************************************************"};

bool loadData(ifstream&, struct Comp[]);
void openFile(char[]);

struct Comp
{
	char name[maxStringLength];
	double time[maxStringLength];
	double sumOfTimes = 0;
	int companyWaitTimeCounter = 0;
};

void openFile(char file[])
{
	char fileName[maxStringLength];
	char fileType[4];

	cout << "Company Call Center Statistical Analyzer" << endl;
	cout << "Enter Call Center Data File: ";
	cin.getline(fileName, maxStringLength, '.');
	cin.getline(fileType, 4, '\n');

	if (strcmp(fileType, "txt") != 0)
	{
		cout << "That is not a valid .txt file. Program terminating\n" << line << endl;
		
	}
	else
	{
		file = strncat(fileName, ".", maxStringLength);
		file = strncat(file, fileType, 4);

		cout << "Thank you, Now opening " << file << ". Please Wait" << endl;
		cout << line << endl;
		
	}


}

bool loadData(ifstream& fileload, struct Comp org[maxCompanies])
{
	if (!fileload.good())
	{
		cout << "File could not be opened" << endl;
		return false;
	}
	else
	{
		cout << "File has opened" << endl;
		while (fileload.getline(org[i].name, maxStringLength, '\t')) //read in company name until \t is found moving on to the times
		{
			cout << org[i].name << " ";
			fileload.ignore();
			while (fileload.good() && fileload >> org[i].time[j]) //read in the doubles until \n is found <<here shaun>>
			{
				cout << org[i].time[j] << " ";
				org[i].sumOfTimes += org[i].time[j];
				org[i].companyWaitTimeCounter++;
				j++;
			}
			fileload.clear();
			cout << "\n Total wait time for Company " << org[i].name << " is: " << org[i].sumOfTimes << endl;
			cout << "With a total of: " << org[i].companyWaitTimeCounter << " times counted" << endl;
			cout << line << endl;
			
			i++;
			j = 0;

		}
		return true;
	}
}

int main(int argc, char const *argv[])
{
	char file[maxStringLength];
	Comp record[maxCompanies];
	

	
	openFile(file);
	
		cout << file << endl;
		ifstream fileInput;
		fileInput.open(file);
		loadData(fileInput, record);
		fileInput.close();
	

	return 0;
}

[TheIdeasMan]

Hi,


What is the value of the variable file when you call the function on line 91?

Always initialise your variables to something.

Avoid global variables when a beginner.

With the layout of your file, have the function declarations first, followed by main(), then function definitions.

Avoid line 4, Google to see why that is so.

Hope all is well :+)

[MrBeavis]

hello,

ok so file should be empty and then filled in the function, are you suggesting I fill it with just a space or something of the sort?

Ill happily change the layout thats how i prefer it too but the lecturer does it this way, fortunately this is revision so :P to him.

are you saying to not put using namespace std?

[rcast]

Could it be That file is never initialized? On line 91, you create a copy of file for openfile function. Parameters to functions create a copy of the variable to work with which goes out of scope when the function call finishes.

http://www.cplusplus.com/doc/oldtutorial/functions2/

If you want to , follow theideasman suggestion and make openFile function return char[] file then call it like so

char[maxStringLength] file = openFile();

And return file from within openFile method.

Or you can pass a reference to file from Main like so

// function definition
void openFile(char[]& file){...}

//call method
openFile(file);

[ne555]

> make openFile function return char[] file then call it like so
> char[maxStringLength] file = openFile();
afaik, you can't do that in c++


> On line 91, you create a copy of file for openfile function
no, you pass the memory address of the first element of the array.
Then thing is, in your function you lose that address at line 39

Use

		strncat(fileName, ".", maxStringLength);
		strncat(fileName, fileType, 4);
		strcpy(file, fileName);

By the way, you are missing #include <cstring>

[rcast]

whoops. Sorry

[TheIdeasMan]

Hi,

are you saying to not put using namespace std;?

Yes, because it brings in the entire std namespace into the global namespace, which can cause naming conflicts - which defeats the purpose of having namespaces. The std namespace is huge, it has the entire STL inside it - all those classes, containers, templates, algorithms etc.

The annoying thing is that lots of lecturers and authors do it, and often don't even explain what namespaces are to their students. Ideally, one should put their own code into a namepsace of it's own.

If you look at code posted by some of the experts on this site (JLBorges is just one that come to mind) you will see they always prefix std:: to the many things in the Standard Template Library.

I suppose this is a thing that beginners are allowed to get away with for awhile, until they find out about how to do it properly.

Hope all is going well at your end :+)

[MrBeavis]

Could it be That file is never initialized? On line 91, you create a copy of file for openfile function. Parameters to functions create a copy of the variable to work with which goes out of scope when the function call finishes.

http://www.cplusplus.com/doc/oldtutorial/functions2/

If you want to , follow theideasman suggestion and make openFile function return char[] file then call it like so


char[maxStringLength] file = openFile();

And return file from within openFile method.

Or you can pass a reference to file from Main like so

1
2
3
4
5
// function definition
void openFile(char[]& file){...}

//call method
openFile(file);

ok im confused, as I understood it you could not pass in an array by reference as when you are passing in an array you are essentially only passing in the address of the first element ie a pointer.
I tried it just incase I was wrong and it does not compile....

as for using namespace, i think we are told to do it until we are taught about it correctly which is this year if Im not mistaken

[MrBeavis]

> make openFile function return char[] file then call it like so
> char[maxStringLength] file = openFile();
afaik, you can't do that in c++


> On line 91, you create a copy of file for openfile function
no, you pass the memory address of the first element of the array.
Then thing is, in your function you lose that address at line 39

Use

strncat(fileName, ".", maxStringLength);
		strncat(fileName, fileType, 4);
		strcpy(file, fileName);

By the way, you are missing #include <cstring>

thanks this worked like a charm :)

[dhayden]

You don't even need fileName in openFile, just use the file parameter:

void openFile(char file[])
{
	char fileType[4];

	cout << "Company Call Center Statistical Analyzer" << endl;
	cout << "Enter Call Center Data File: ";
	cin.getline(file, maxStringLength, '.');
	cin.getline(fileType, 4, '\n');

	if (strcmp(fileType, "txt") != 0)
	{
		cout << "That is not a valid .txt file. Program terminating\n" << line << endl;
		
	}
	else
	{
		strncat(file, ".", maxStringLength);
		strncat(file, fileType, 4);
		cout << "Thank you, Now opening " << file << ". Please Wait" << endl;
		cout << line << endl;
	}
}

You have global variables i and j that are only used inside loadData. Make then locals instead and use for loops to make their usage clear:

bool loadData(ifstream& fileload, struct Comp org[maxCompanies])
{
	if (!fileload.good())
	{
		cout << "File could not be opened" << endl;
		return false;
	}
	else
	{
		cout << "File has opened" << endl;
		//read in company name until \t is found moving on to the times
		for (int i=0; fileload.getline(org[i].name, maxStringLength, '\t'); ++i)
		{
			cout << org[i].name << " ";
			fileload.ignore();

			//read in the doubles until \n is found <<here shaun>>
			for (int j=0; fileload.good() && fileload >> org[i].time[j]; ++j) 
			{
				cout << org[i].time[j] << " ";
				org[i].sumOfTimes += org[i].time[j];
				org[i].companyWaitTimeCounter++;
			}
			fileload.clear();
			cout << "\n Total wait time for Company " << org[i].name << " is: " << org[i].sumOfTimes << endl;
			cout << "With a total of: " << org[i].companyWaitTimeCounter << " times counted" << endl;
			cout << line << endl;
		}
		return true;
	}
}