Please check my C++

This is a discussion on Please check my C++ within the C++ Programming forums, part of the General Programming Boards category; Being doing C wanna get involved in C++... Will do this by showing you my Program... I will post two ...

  1. #1
    Registered User
    Join Date
    Apr 2008
    Posts
    610

    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 "
    Last edited by csonx_p; 07-09-2008 at 04:21 AM.

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,914
    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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by laserlight View Post
    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...

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    <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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Question... To access the make & model from Car, do i need public getModel/getMake functions?

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,914
    To access the make & model from Car, do i need public getModel/getMake functions?
    Yes, since they are private member variables.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    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

  8. #8
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    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() .....

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  10. #10
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    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!

  11. #11
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by csonx_p View Post
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  12. #12
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,782
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  13. #13
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by Elysia View Post
    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...

  14. #14
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,782
    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;
    }
    Last edited by Elysia; 07-10-2008 at 01:02 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  15. #15
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by Elysia View Post
    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?

Page 1 of 18 1234567891011 ... LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. BN_CLICKED, change button style
    By bennyandthejets in forum Windows Programming
    Replies: 13
    Last Post: 07-05-2010, 11:42 PM
  2. how to check input is decimal or not?
    By kalamram in forum C Programming
    Replies: 3
    Last Post: 08-31-2007, 07:07 PM
  3. Please check this loop
    By Daesom in forum C++ Programming
    Replies: 13
    Last Post: 11-02-2006, 12:52 AM
  4. A way to check for Win98 or WinXP
    By Shadow in forum A Brief History of Cprogramming.com
    Replies: 5
    Last Post: 10-31-2002, 10:06 AM
  5. how to check for end of line in a text file
    By anooj123 in forum C++ Programming
    Replies: 6
    Last Post: 10-24-2002, 11:21 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21