Printing String to textfile produces (null)

[Tigjaw]

Hi all,
I'm new here and quite new to C++ so be gentle!
I have a struct which contains several variables including string, float and int. I create an array of 6 elements of this struct and populate those elements by reading the data from a text file. I then wish to print the array elements back into a new file. Here is the textfile that is being read from:

Renault_4s,300,70,700,800,2500
Alfa_Romeo_147,160,129,800,900,4000
Aston_Martin_DB9,60,190,500,450,6500

This is all working perfectly as far as I can tell except...this is the file that has been created and written to:

(null),300.000000,70,700.000000,800.000000,2500.000000
(null),160.000000,129,800.000000,900.000000,4000.000000
(null),60.000000,190,500.000000,450.000000,6500.000000

Why (null)?

Here is my program code:

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

using namespace std;

void ReadFile();

FILE *write;

struct Car {
	string CarName;
	float Acceleration;
	int TopSpeed;
	float TempMod;
	float OilMod;
	float RpmMod;

	Car() {
		// Default Constructor
		CarName = "Default Car";
		Acceleration = 200;
		TopSpeed = 100;
		TempMod = 500;
		OilMod  = 600;
		RpmMod = 3500;
	};

};

Car cars[6];

int _tmain(int argc, _TCHAR* argv[])
{
	ReadFile();
	return 0;
}

void ReadFile() {
	FILE *read;

	int c;
	// open file to write/append to new/existing file
	// this file is used for testing purposes
	if((write = fopen ("testingcars.txt", "w")) == NULL) {
		printf("Error Creating File.\n");
		getchar();
		exit(1);
	}
	fprintf(write, "testingcars.txt, Error and Feedback log, testing cars.txt:\n\n");
	// opens file, if can't open, moan about it
	if((read = fopen ("cars.txt", "r")) == NULL) {
		//printf("Cannot open file.\n");
		fprintf(write, "Error opening cars.txt");
		getchar();
		exit(1);
    }
	while( (c = fgetc(read)) != EOF) {
		fprintf(write, "%c", c);
	}
	fprintf(write, "\n\nArray Data:\n\n");
	string Name;
	float Acc;
	int Top;
	float Temp;
	float Oil;
	float Rpm;
	ifstream fin("cars.txt");
	string line;
	int CountCars = 0;
	
	while (getline(fin, line)  && CountCars <6) {
		cout << line << endl;
                istringstream tokenizer(line);
                string token;

		getline(tokenizer, token, ',');
		istringstream s_iss_Name(token);
		s_iss_Name >> Name;
		cars[CountCars].CarName = Name;
		cout << cars[CountCars].CarName << endl;
		fprintf(write, "%s,", cars[CountCars].CarName);

		getline(tokenizer, token, ',');
                istringstream f_iss_Acc(token);
                f_iss_Acc >> Acc;
		cars[CountCars].Acceleration = Acc;
                //cout << cars[CountCars].Acceleration << endl;
		fprintf(write, "%f,", cars[CountCars].Acceleration);

		getline(tokenizer, token, ',');
                istringstream int_iss(token);
                int_iss >> Top;
		cars[CountCars].TopSpeed = Top;
		//cout << cars[CountCars].TopSpeed << endl;
		fprintf(write, "%d,", cars[CountCars].TopSpeed);

		getline(tokenizer, token, ',');
                istringstream f_iss_Temp(token);
                f_iss_Temp >> Temp;
		cars[CountCars].TempMod = Temp;
		//cout << cars[CountCars].TempMod << endl;
		fprintf(write, "%f,", cars[CountCars].TempMod);

		getline(tokenizer, token, ',');
                istringstream f_iss_oil(token);
                f_iss_oil >> Oil;
		cars[CountCars].OilMod = Oil;
                //cout << cars[CountCars].OilMod << endl;
		fprintf(write, "%f,", cars[CountCars].OilMod);

		getline(tokenizer, token, ',');
                istringstream f_iss_rpm(token);
                f_iss_rpm >> Rpm;
		cars[CountCars].RpmMod = Rpm;
                //cout << cars[CountCars].RpmMod << endl;
		fprintf(write, "%f\n", cars[CountCars].RpmMod);
		
		CountCars++;
	}
	fclose(write);
	fclose(read);

}

The complete testingcars.txt output:

testingcars.txt, Error and Feedback log, testing cars.txt:

Renault_4s,300,70,700,800,2500
Alfa_Romeo_147,160,129,800,900,4000
Aston_Martin_DB9,60,190,500,450,6500

Array Data:

(null),300.000000,70,700.000000,800.000000,2500.000000
(null),160.000000,129,800.000000,900.000000,4000.000000
(null),60.000000,190,500.000000,450.000000,6500.000000

I'd really appreciate any help, I've been at this all day and just can't get the names to print into this file. This isn't even part of my assignment so I can ignore it if need be but I'd love to get it working for my own sake.

Thanks in advance and let me know if I can provide any more info,
Kind regards and a happy new year,

Josh

P.S: Visual Studio 2008 Professional if that is important to know.

[Null]

I think problem is in line 17 because CarName is type of "string" but it should be C style string.

[Tigjaw]

Hey thanks for the reply,

So is there any workaround for this? Will sprintf do what I want? I've tried sprintf in place of the fprintf statement in question but compilation failes due to incorrect parameters.

[guestgulkan]

Why are you mixing C and C++ file functions??

There's no law against it - but...why?

[Tigjaw]

Ah I don't know, I'm just learning, I only began learning C (ANSI) about a month ago and C++ a few weeks ago so I'm still mixing my methods up.

I understand it may be bad practice to do this but I hope - considering we've only been taught ansi C file IO - that at least for now it wont hurt my case.

Believe me, it could be much worse, I could still be trying to write java in my c programs! :)

[moorecm]

I think your problem is on line 87:

fprintf(write, "%s,", cars[CountCars].CarName);

cars[CountCars].CarName is a string, and fprintf's %s format expects a C-style string. std::string has a c_str() method that returns a const char *.

Try this:

fprintf(write, "%s,", cars[CountCars].CarName.c_str());

I'm guessing that will fix it up. However, I strongly suggest sticking with just fstream for file access unless you absolutely have to resort to FILE pointers and their related functions.

I would like to see this code cleaned up with some encapsulation and a few methods to reduce the somewhat redundant parts.

[Tigjaw]

Hi moorecm, thanks for the advice, as you can see I'm still very much a newbie at c and c++ so I appreciate all your pointers <chuckles at the nerdy pun I just made>.

Your suggestion worked perfectly thanks very much.

By encapsulation, do you mean - as an example - encapsulating some of the variables in the ReadFile() method into an object or instance of a class or struct? I do realise there's some unnecessary redundancy, usually when I'm programming especially with java with which I am much more adept I'm very good at producing clean code but learning ANSI c, c++ and directx all in the space of one month have really put my coding puntuation on the back burner.

[moorecm]

No problem.

By encapsulation, I am refering to making your Car member data private rather than public. You could also, for example, create a Car object and have a method to read its data from disk. To reduce the size of the function to read from disk, you could define a few private methods that read a field and convert it to a specific type. I see the blocks reading a field and using a string stream to convert the data as somewhat repetitious. (BTW, you might want to take a good look at lines 83-85 and ask yourself what they are doing.)

[Tigjaw]

You'll be glad to know I removed lines 83-85! I think I wrote them when I was maniacally attempting to get fprintf to print my string, not sure why I thought they would work.

I am refering to making your Car member data private rather than public

Are you talking about the variables in the car struct? and if so, isn't that the idea of a struct? ie, that its variables are public by default? Would this suggestion be qualified if I were to simply change the car struct into a class and produce setters and getters instead of modifying the variables directly elsewhere in the application? (This also presuming my assumption that a class's variables are private by default is true)

To reduce the size of the function to read from disk, you could define a few private methods that read a field and convert it to a specific type

Are you suggesting these private methods be methods of the car struct/class or simply be private methods in the application?

I'm assuming the suggestion of privatising variables and methods is simply to enforce data integrity?

[moorecm]

Are you talking about the variables in the car struct? and if so, isn't that the idea of a struct? ie, that its variables are public by default? Would this suggestion be qualified if I were to simply change the car struct into a class and produce setters and getters instead of modifying the variables directly elsewhere in the application? (This also presuming my assumption that a class's variables are private by default is true)

Yes and yes. Although, a struct is a user-defined type practically identical to a class (excluding the default access specifier). I would not be surprised find an example of when a struct is appropriate but I can't say that I have ever encountered one. As far as I am concerned, encapsulation is a good thing and should be promoted. I've been in a few arguments for using structs in my professional experience and, to be perfectly objective about it, I could summarize by saying the developer was being lazy.

As far as the getters and setters, they may not all be necessary. Only information that a client outside of the class would need a getter and only information that a client outside of the class should be able to change would need a setter.

Are you suggesting these private methods be methods of the car struct/class or simply be private methods in the application?

Of the class, of course! The function is already tightly coupled to the class, as it requires a specific implementation of Car to work. Note that if ReadFile was a method of the class, it would not need to use the getters or setters, it could access the member data directly.

I'm assuming the suggestion of privatising variables and methods is simply to enforce data integrity?

Mainly. It also serves as a nice grouping/modeling mechanism. I prefer OOP until there is a specific reason to break out of the usual--I think it's a good practice to "default" to OOD/OOP. This is, of course, just my opinion/preference.

[Tigjaw]

Thanks again moorecm. When we've been taught java, OOP was thrown at us as mandatory. In C++ however, OOP has been introduced as an option. To say the least this did confuse me although after investigating OOP with c++ perhaps we haven't been taught it yet as it isn't quite as 'newbie proof' as OOP with java. Especially with the ability of creating classes or structs within a 'class' itself. By that I mean, in java one document is a class and one class has its own document whereas in C++ a class is defined within a document. Furthermore this same document can contain many classes and structs.

It's all quite new to be honest. Nevertheless, my preference is still OOP nomatter the language as it just makes sense to me so your explanations (as above) are very much appreciated, it's better to have a clue than to have a guess and guessing is what I've been doing so far! I forgot to ask "when is a struct appropriate" and you answered that too :)