Please check my C++

Being doing C wanna get involved in C++... Will do this by showing you my Program... I will post two files first to try avoid confusion... After these files has been scrutinized and corrected, will post the next file... & so forth... I will also include the errors am getting

TITLE & Description: To Read from file & List a fleet of cars, and update them
Program Usage: Car Rental Shop

First File ... Class Car

Car.h

Code:

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

#ifndef DATE
#define DATE 1

struct Date						
{
	unsigned day;
	unsigned month;
	unsigned year;
};

#endif

class Car						
{
	
public: 
	
	Car();						
	
	enum TankStatus {unknown, empty, half, full};	 
	static std::string getTankStatusName(TankStatus);
	bool sameModel(const std::string&) const;
	bool operator<(const Car&) const;	
	void read(FILE *, bool);
	void write(FILE *, bool);
	void setDatesAndTank(Car&);
	CarFleetIterator find(CarFleetIterator&, CarFleetIterator&, std::string);

private:

	std::string	make;			// Make as in..."ford"
	std::string	model;			// Model as in ... "icon"
	unsigned	engineCapacity;	          // Engine capacity may be stored as "1800" not 1.8
	unsigned	yearModel;			
	Date		returnDate;			
	TankStatus	tankStatus;		// Maybe filled, half-filled, or empty
};

Q : Does this class make sense? should read/write member functions be there...
Errors :
1. <fstream.h> No such file or directory ??? Using VS 2005
2. missing ';' before identifier 'find' ... Find defined below in Car.cpp


Car.cpp

Code:

#include "car.h"

Car::Car() {};	

std::string Car::getTankStatusName(TankStatus status)
{
	switch (status)
	{
		case unknown: 
			return "unknown";
			break;
		case empty: 
			return "Empty";
			break;
		case half: 
			return "Half-Empty";
			break;
		case full: 
			return "Full";
			break;
		default:
			throw std::runtime_error("Invalid tank status");
			return "";
	} 
}

// Operator < will be used by sort() function 

bool Car::operator<(const Car& car) const
{
	return make < car.make;
}


// Loading data from the text file

void Car::read(FILE *fin, bool file_out)
{
	char make_temp[15], model_temp[15];

	if(file_out)	// Get record from file
	{	
		fscanf(fin, "&#37;s%s", make_temp, model_temp);
		fscanf(fin, "%d", &yearModel);
		fscanf(fin, "%d", &engineCapacity);
		fscanf(fin, "%d%d%d", &returnDate.day, &returnDate.month, &returnDate.year);
		fscanf(fin, "%d", &tankStatus);
		make = make_temp;
		model = model_temp;
	}
	else	// Get record from user
	{
		fscanf(fin, "%s%s", make_temp, model_temp);
		fscanf(fin, "%d", &yearModel);
		fscanf(fin, "%d", &engineCapacity);
		fscanf(fin, "%d", &tankStatus);
		make = make_temp;
		model = model_temp;

		// No dates  yet!
		returnDate.day  = returnDate.month = returnDate.year =0;
	}
}

// Saving data to the text file

void Car::write(FILE *fout, bool file_out)
{
	char make_temp[15], model_temp[15];

	strcpy(make_temp, make.c_str());
	strcpy(model_temp, model.c_str());

	if(file_out)	// To the output screen
	{
	
		fprintf(fout, "%-9s%-11s", make_temp, model_temp);
		fprintf(fout, "%-12d", yearModel);
		fprintf(fout, "%-10d", engineCapacity);
		fprintf(fout, "%-3d%-3d%-8d", returnDate.day, returnDate.month, returnDate.year);
		fprintf(fout, "%-4s", getTankStatusName(tankStatus).c_str());

		puts("");
	}
	else			// To the file
	{
		fprintf(fout, "%s %s ", make_temp, model_temp);
		fprintf(fout, "%d ", yearModel);
		fprintf(fout, "%d ", engineCapacity);
		fprintf(fout, "%d %d %d ", returnDate.day, returnDate.month, returnDate.year);
		fprintf(fout, "%d", tankStatus);
	}
}

// Comparing the user entry with current record

bool Car::sameModel(const std::string& car_model) const
{
	return model == car_model;	
}

// Allow Dates and tank to be modified

void Car::setDatesAndTank(Car& car_dates_tank)
{
	cout << "\nEnter return dates [DAY - MONTH - YEAR]" << endl;
	cin >> car_dates_tank.returnDate.day;
	cin >> car_dates_tank.returnDate.month; 
	cin >> car_dates_tank.returnDate.year;

	cout << "\n\nEnter tank status [1...3]";
	scanf("%d", car_dates_tank.tankStatus);

	/* 
	try {
		cin >> car_dates_tank.tankStatus;
	} 
	catch ( out_of_range& ex ) {
		cerr<< ex.what() <<endl;
	}
	*/
}

Menu::CarFleetIterator Car::find(CarFleetIterator& b, CarFleetIterator& e, string s)
{
	int pos;

	Car& car = *b;
	
	// Find the make from the list from begin to end

	while (b <= e)
	{	
		if( pos = strcmp(s, make) )
		{ 
			return b;             
		}
		else {
			b++;
		}
	} 

	return e;
}

Lastly, Menu.h

Code:

// Purpose	: To declare a class Menu

#pragma once

#include <vector>
#include "Car.h"

/* Menu class acts as a Main controller */

class Menu
{

public:

	Menu();

	void run();

private:

	enum Options {DESCRIBE = 49, READF, DISPLAY, SEARCHF, ADDCAR, UPDATEF, EDITF};	// Menu Options
	typedef std::vector<Car> CarFleet;
	typedef CarFleet::iterator CarFleetIterator;

private:

	void pause();
	void clrscr();
	void sortAlpha();
	void AddNewCar();
	void EditFleet();
	void programLimits();
	void titleDescription();
	void updateFleetFile();
	void readFleetFromFile();
	CarFleetIterator searchCarDetails(bool&);
	void showFleetList(CarFleetIterator, bool);

private:

	CarFleet fleet; 
};

NOTE: In Car.h i included Menu.h, then in Menu.h i included Car.h... I think there's a problem there... But i need the vector declared in Menu.h within Car class & the Car object within Menu.h ... How do i resolve this error " 'Car' : 'class' type redefinition "

But i need the vector declared in Menu.h within Car class

From what I see, that is not true. You just need CarFleetIterator, but the find() member function does not belong in Car anyway, so remove it and your problem is solved. If you need it, declare it in Menu.h as a free function.

Originally Posted by laserlight

From what I see, that is not true. You just need CarFleetIterator, but the find() member function does not belong in Car anyway, so remove it and your problem is solved. If you need it, declare it in Menu.h as a free function.

Wow! that reduced a lot of errors, now stuck with the fstream problem.... cin & cout gets compile errors...

<fstream.h> is old-style C++ headers. You should be using <fstream> without the .h

You probably need "std::cin" and "std::cout" if you don't have "using namespace std".

The variable name of "file_out" is a bit misleading in your "read" function. I also wonder why you are using fscanf() on a "FILE *", when you really should be using the stream functions for proper C++?

Amusing co-incidence: I wrote a commercial car rental package in Pascal about 20 years ago.

--
Mats

Question... To access the make & model from Car, do i need public getModel/getMake functions?

To access the make & model from Car, do i need public getModel/getMake functions?

Yes, since they are private member variables.

Trying to rewrite the following in C++

Code:

FILE *fout;

fprintf(fout, "%-9s%-11s", make_temp, model_temp);

known that FILE *fout will be ofstream....

i.e.

Code:

ofstream fout;

fout << ???????

currently looking at setw

Originally Posted by matsp

I also wonder why you are using fscanf() on a "FILE *", when you really should be using the stream functions for proper C++?
Mats

Problem..

Code:

fin >> tankStatus;

error C2679: binary '>>' : no operator found which takes a right-hand operand of type 'Car::TankStatus' (or there is no acceptable conversion)

Now i remember, this the reason i went back to fscanf() .....

You don't "have" to use setw. But it may look nice. Just putting a space between the two strings would work.

As your fprintf()/fscanf() combinations stand, if your car is called "Chevrolet" as make, there will be no space between that and the model, and the read function would merge the make with the model, and overflow your variable...

--
Mats

Originally Posted by matsp

You don't "have" to use setw. But it may look nice. Just putting a space between the two strings would work.

As your fprintf()/fscanf() combinations stand, if your car is called "Chevrolet" as make, there will be no space between that and the model, and the read function would merge the make with the model, and overflow your variable...

--
Mats

Any ideas about the '>>' error i'm getting stated in the thread above!

Originally Posted by csonx_p

Any ideas about the '>>' error i'm getting stated in the thread above!

Well, technically, I believe it's incorrect to read an enum as an integer, so your "C" solution is also invalid - even if it happens to work in this case.

You could use a temporary integer, and then assign that to the enum (for output, you could just use

Code:

reinterpret_cast<int>(tankstatus)

.

My method would probably involve using the getTankStatusName and the a new function that does the reverse thereof.
[Actually, I would probably represent tank status as an integer in either liters [but that's hard to determine for the renter] or in 1/8th units of full (so 0 is empty, 1 is nearly empty, 7 nearly full and 8 is full - with perhaps 9 or -1 as "unknown") - it was not a problem I had to solve in my car rental program, as the rule was that you got the car full and returned it full - if it wasn't full when you returned it, you'd get billed for the cost of filling, at approx 1.5x the price of the fuel, if memory serves]

--
Mats

Streams do not understand your custom type, so it complains if you try to read anything to and from those.
There's a solution for types of enums, but that involves specializing templates (at least the only "C++" way to my knowledge), which you probably don't know yet, so I would go for mats's solution for now.

read/write should be replaced with overloaded operators for << and >>.
And you should stop trying to strip variable names from the class definitions. It's bad practice.
And if you to be completely correct, then you class lacks const correctness.

Originally Posted by Elysia

Streams do not understand your custom type, so it complains if you try to read anything to and from those.
There's a solution for types of enums, but that involves specializing templates (at least the only "C++" way to my knowledge), which you probably don't know yet, so I would go for mats's solution for now.

read/write should be replaced with overloaded operators for << and >>.
And you should stop trying to strip variable names from the class definitions. It's bad practice.
And if you to be completely correct, then you class lacks const correctness.

Elysia, in most cases you have interesting suggestions and well appreciated too but you always assume i/person understand the terminology, or the ideas you put... I like learning new stuff and solving things different ways but examples explaining your suggestions would help. Even if its' simple code or links..

PS i have no idea what you suggestion.... Please expand...

Sorry, let me make it a bit clearer:

Code:

class Car						
{
	
public: 
	
	Car();						
	
	enum TankStatus {unknown, empty, half, full};	 
	static std::string getTankStatusName(TankStatus);
	bool sameModel(const std::string&) const;
	bool operator<(const Car&) const;	
	void read(FILE *, bool); // Remove
	void write(FILE *, bool); // Remove
	void setDatesAndTank(Car&);
	CarFleetIterator find(CarFleetIterator&, CarFleetIterator&, std::string);

private:

	std::string	make;			// Make as in..."ford"
	std::string	model;			// Model as in ... "icon"
	unsigned	engineCapacity;	          // Engine capacity may be stored as "1800" not 1.8
	unsigned	yearModel;			
	Date		returnDate;			
	TankStatus	tankStatus;		// Maybe filled, half-filled, or empty
};

The red functions should probably be removed.
Then you can overload the >> and << operators (you already overloaded operator <, so you should understand operator overloading, right?).
That means you can do:

Car mycar;
mystream << car;
mystream2 >> car;

Which is a much more preferred C++ way.
And the prototypes:

std::string Car::getTankStatusName(TankStatus status)

^ This is your implementation, but your definition is:

static std::string getTankStatusName(TankStatus);

You're leaving out the actual name (seen in red). It's better if you don't.

As for the const correctness... it means that you should put the const keyword at proper places. You've put it in some places, but not all. You should probably try to expand on that. Remember that functions can be const too!


Actually, I just tested and realized you don't need templates (oops). Anyway, here's an example of how you can hide implementation:

Code:

namespace std
{
	ostream& operator << (ostream& rLhs, Car::TankStatus eStatus)
	{
		switch (eStatus)
		{
			case Car::Ready:
				rLhs << "Ready";
				break;
			case Car::Destroyed:
				rLhs  << "Destroyed";
				break;
		}
		return rLhs;
	}

	istream& operator >> (istream& rLhs, Car::TankStatus& rRhs)
	{
		rLhs >> reinterpret_cast<int&>(rRhs);
		return rLhs;
	}
}

int main()
{
	Car::TankStatus status;
	cout << status;
	cin >> status;
}

Originally Posted by Elysia

Sorry, let me make it a bit clearer:

Actually, I just tested and realized you don't need templates (oops). Anyway, here's an example of how you can hide implementation:

Code:

namespace std
{
	ostream& operator << (ostream& rLhs, Car::TankStatus eStatus)
	{
		switch (eStatus)
		{
			case Car::Ready:
				rLhs << "Ready";
				break;
			case Car::Destroyed:
				rLhs  << "Destroyed";
				break;
		}
		return rLhs;
	}

	istream& operator >> (istream& rLhs, Car::TankStatus& rRhs)
	{
		rLhs >> reinterpret_cast<int&>(rRhs);
		return rLhs;
	}
}

int main()
{
	Car::TankStatus status;
	cout << status;
	cin >> status;
}

Below was my attempt of overloading before i looked at your post

Code:

// Define operator '>>' for input streams 'cin' to read enum type

istream& Car::operator>> (istream& in, TankStatus val) const
{
	int val;
	in >> val;
	return in;
}

After compiling, i got an error...

Code:

error C2804: binary 'operator >>' has too many parameters

Now input stream is is used not just for TankStatus (or enum type)... Does overloading for one type affect the other input types in the program?