Please check my C++

**csonx_p** · 07-14-2008

Originally Posted by Elysia

Yeah, that looks like a date class, but sometimes you might not want to specify everything, such as perhaps minutes and seconds.
You could make them optional

While we on date ..

Code:

void Menu::setReturnDates(const Date& dates)
{
	int day, month, year;

	cout << "\nEnter return dates [DAY - MONTH - YEAR]" << endl;
	cin >> day;
	cin >> month; 
	cin >> year;
	
	dates.setTimeDate(day, month, year, 0, 0, 0);
}

Q.

1. Should i make Date const, got an error

Code:

 error C2662: 'Date::setTimeDate' : cannot convert 'this' pointer from 'const Date' to 'Date &'

2. sometimes when i type dates + '.' the members list does not come up (or drop down), should i be worried? I mean is date class visible even in that case? setReturnDates is a member of Menu.

**matsp** · 07-14-2008

You should never have const reference when you intend to call setters.

As Elysia suggested, perhaps you'd like to use default values for the h, m and s parameters so that you don't have to give a bunch of zero's.

--
Mats

**Elysia** · 07-14-2008

Originally Posted by csonx_p

1. Should i make Date const, got an error

Code:

 error C2662: 'Date::setTimeDate' : cannot convert 'this' pointer from 'const Date' to 'Date &'

Because setTimeDate is not const and you can only call const functions from an object that is const. Since a setter can never be const, you should not take the argument by const reference.

2. sometimes when i type dates + '.' the members list does not come up (or drop down), should i be worried? I mean is date class visible even in that case? setReturnDates is a member of Menu.

Sometimes it screws up. And sometimes it screws up due to an error in your code.
If it happens, try compiling the current code and fixing the mistakes.

**csonx_p** · 07-14-2008

Originally Posted by matsp

As Elysia suggested, perhaps you'd like to use default values for the h, m and s parameters so that you don't have to give a bunch of zero's.
Mats

By this you referring to getting the current time from system maybe? Or just set to any hour, minute... I removed the seconds, no relevancy to this program really

**matsp** · 07-14-2008

Originally Posted by csonx_p

By this you referring to getting the current time from system maybe? Or just set to any hour, minute... I removed the seconds, no relevancy to this program really

I mean you set hour and minute (and second?) to zero if they are not specified.

--
Mats

**csonx_p** · 07-14-2008

Code:

// Overload for reading input streams
istream& operator >> (istream& in, Car& car)
{
	string make, model;
	int ymodel, engCap;
	TankStatus ts;

	in >> make >> model;
	in >> ymodel;
	in >> engCap;
	in >> ts;
	
	car.setCarDetails(make, model, ymodel, engCap, ts);
}

My thoughts are the overloaded function should only have the four lines, not the setCarDetails()...

here's the setCarDetails

Code:

void Car::setCarDetails(string make, string model, unsigned yrModel, 
						unsigned engCap, TankStatus tnkStatus)
{
	make = carMake;
	model = carModel;
	yearModel = yrModel;
	engineCapacity = engCap;
	tankStatus = tnkStatus;
}

OR, can overloaded operators call other functions within?

**Elysia** · 07-14-2008

There's no restrictions to what operators can call.

**matsp** · 07-14-2008

Your code is correct.

If you want to operate directly on car, you either need to make the operator>> function a friend ("friends" are trusted to not do silly things with an object, and thus can see private member variables and functions), or make it a member function of the object itself.

Whether you prefer to call a member setter function or access the member variables directly is pretty much a design decision that can be argued either way, and without any particular knowledge of the overall design/requirements, it would be impossible to say which is better. At this point, you decide what you want to do. The call of the setter has the benefit of being safer - but potentially slower because of the call overhead. I don't think performance (here at least) will be a factor, so I wouldn't worry about that bit.

--
Mats

**csonx_p** · 07-14-2008

Thnx mats & Elysia for your help... There's a progress...

Now i've compiled after the changes which includes the use of the '<<' & '>>' overloaded operators.. I'm gonner post them by step to avoid confusions...

read member function

Code:

void Car::read(istream& indata, Car& car, bool isKeyboard)
{
	indata >> car;
	if(isKeyboard) {
		readDates(indata);
	}
}

it's declaration in class Car

Code:

void read(istream& i, const Car&, bool t);

error..

Code:

error C2511: 'void Car::read(std::istream &,Car &,bool)' : overloaded member function not found in 'Car'

?????

**Elysia** · 07-14-2008

You forgot to change your class definition to match your new implementation.
You made a mistake somewhere.
And a tip for you: name your prototypes exactly as your implementation.
Ie:

void Car::read(istream& indata, Car& car, bool isKeyboard)
And
void read(istream& indata, Car& car, bool isKeyboard);

Makes it more readable.
(As you will see, Visual Studio intellisense gets its information from your definition.)

**csonx_p** · 07-14-2008

Code:

// Declare class to hold car details

#include <string>
#include <fstream>
#include <iostream>
#include <iomanip>
#include "RentalDate.h"

using namespace std;

class Car						
{
public: 
	
	Car();						
	
	enum TankStatus {unknown, empty, half, full};	 

	void setCarDetails(string m,string ml,unsigned yrm,unsigned engCap,TankStatus ts);
	static string getTankStatusName(TankStatus);
	bool sameModel(const string&) const;
	bool operator<(const Car&) const;	
	void read(istream&, const Car& c, bool);
	void write(ostream&, bool);
	
	string getModel();
	string getMake();
	unsigned getEngineCap();
	unsigned getYearModel();
	TankStatus getTankStatus();

private:

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

// Overload fstream operators for enum variable
ostream& operator << (ostream& out, Car::TankStatus eStatus);
istream& operator >> (istream& in, Car::TankStatus& eStatus);
istream& operator >> (istream& in, Car& car);
void readDates(Date& dates);

**matsp** · 07-14-2008

Originally Posted by csonx_p

Thnx mats & Elysia for your help... There's a progress...

Now i've compiled after the changes which includes the use of the '<<' & '>>' overloaded operators.. I'm gonner post them by step to avoid confusions...

read member function

Code:

void Car::read(istream& indata, Car& car, bool isKeyboard)
{
	indata >> car;
	if(isKeyboard) {
		readDates(indata);
	}
}

it's declaration in class Car

Code:

void read(istream& i, const Car&, bool t);

error..

Code:

error C2511: 'void Car::read(std::istream &,Car &,bool)' : overloaded member function not found in 'Car'

?????

Hmm. Why would you have a function that reads into a car object, that in itself is declared (and not static) in the Car class. I would either make it a free function (not part of the class), or use the current object (this) as the car object, and not pass it.

I don't know why it's going wrong - perhaps you need std::istream in your class member function?

Edit: And your problem in itself is that in your declaration in the class, you use const Car &, then loose the const in the actual function definition - you should not have const in either place in this instance, as you INTEND to modify car.

--
Mats

**Elysia** · 07-14-2008

error C2511: 'void Car::read(std::istream &,Car &,bool)' : overloaded member function not found in 'Car'

You sure that the ONLY error you get?

**csonx_p** · 07-14-2008

Originally Posted by Elysia

You sure that the ONLY error you get?

NOOOO! that was the first, but now got different ones after modifications...

First, modifications

Code:

void Car::read(istream& indata, Car& car, bool isKeyboard) // removed const before car
{
	Date dates; // added

	indata >> car;
	if(isKeyboard) {
		readDates(dates); // changed argument from stream to dates
	}
}

New errors...

1.

Code:

// Overload for enum variable when writing data
ostream& operator << (ostream& out, Car::TankStatus enumTStatus)
{
	out << reinterpret_cast<int&>(enumTStatus);
	return out;
}

// Error 1
: error C2084: function 'std::ostream &operator <<(std::ostream &,Car::TankStatus)' already has a body

2.

Code:

// Ouput the data to screen or file
void Car::write(ostream& out, bool isScreen, Date& date)
{
	if(isScreen)	// To the output screen
	{
		out << setw(6) << make << setw(8) << model;
		out << setw(10) << yearModel;
		out << setw(12) << engineCapacity;
		out << setw(9) << date.day << setw(3) << date.month << setw (5) << date.year;
		out << setw(13) << getTankStatusName(tankStatus).c_str();
		puts("");
	}
	else			// To the file
	{
		out << " " << make << " " <<  model;
		out << " " << yearModel;
		out << " " << engineCapacity;
		out << " " << returnDate.day << " " << returnDate.month << " " << returnDate.year;
		out << " " << tankStatus;
	}
}

// Error 2
: error C2511: 'void Car::write(std::ostream &,bool,Date &)' : overloaded member function not found in 'Car'

i'll pause here...

EDIT: Picked up some of my probs in red, and, am not using getters where am suppose to

**matsp** · 07-14-2008

the function write has no date in the Car declaration, it has in your write function defintion.

Code:

	out << reinterpret_cast<int&>(enumTStatus);

Are you sure that you want to cast it to int&?

Are you sure the problem with operator>> on tankstatus isn't that you are declaring two outputs instead of one input and one output?

--
Mats