Thread: Is this a good example of OOP?

this will help more people to help you:

Code:

#include <iostream>
#include <fstream.h>
#include <conio.c>
#include <time.h>
#include <string>

class Car
{
 public:
 Car();  // Constructor
 ~Car();  // Destructor
 void CreateCar();  // A more user-friendly approach

 protected:
 string name;    // ie: Honda, Civic, Ford, etc.
 string type;    // ie: Minivan, truck, van, etc.
 unsigned short int year;  // ie: prompts the user to input four digit(ie: 1986) as oppose to 2(ie: 86)
 unsigned short int engine; // ie: Maximum MPH(Miles Per Hour) of car
 string MPH; // prints out MPH
};

Car::Car()
{
 year;
 engine;
 MPH="MPH";
}

Car::~Car()
{
}

void Car::CreateCar()
{
 cout << "Input the name of your automobile:(ie: Honda, Civic, etc.) ";
  cin >> name;
 cout << "\nInput the type of car:(ie: minivan, basic automobile, van, truck, etc.) ";
  cin >> type;
 cout << "\nInput the year are automobile was created:(ie: 1986, 1998, etc.) ";
  cin >> year;
 cout << "\nInput your automobile's maximum miles per hour(MPH): (ie: 234, 252) ";
  cin >> engine;

  ofstream car("Car.txt");
  car << "Name: " << name << endl;
  car << "Type: " << type << endl;
  car << "Model: " << year << ", " << name << endl;
  car << "Speed: " << engine << MPH << endl;
  car.close();
}

int main()
{
 Car database;
 database.CreateCar();
 getch();
 return 0;
}

the database of cars is supposed to contain more than one car i assume, so you'd make it an array of cars instead of just one car.

Your constructor is quite odd. It looks like you aren't declaring anything as anything.. i've never seen anyone attempt that before. I bet that won't even compile.
The variables are initialized when an object of the car class is created, so you don't need to declare them in the constructor.

eh.. that's all i feel like writing for now.. maybe later i'll do something more productif with this.

Actually, it does compile. That is a very wierd constructor though, you don't need one in your app, with the exception of the string MPH which you could just initialize in the class declaration.

And as per the array of cars, I assume you mean an array of objects when you say an array of cars.( Just trying to understand what your saying)

aye, that was what i meant... an array of car objects.

Having a default constructor that stops program execution for keystrokes (via cin) is usually a bad arrangement for OOP, because often an object would need to be declared before it could be loaded with data. Additionally, hardcoding cin as the istream is not good OO programming. However, utilizing an undereffective constuctor and creating a mutator to serve as a constructor is not good either. A better solution might be providing three constuctors:

Car::Car();
//Takes nothing, initializes variables as null strings (in case of an array declaration)

Car::Car(istream&);
//Reads unprompted from an istream (for a data file)

Car::Car(ostream&, istream&);
//Prompts for (on ostream) and accepts (on istream) interactive user input
//Pre: ostream is tied to istream (practical applications cin+cout or wcin and wcout)

What you are aiming for with OO programming is the ability to work with objects of type Car in any context, so you should clearly separate constructors from mutators and accessors.

Also, in your CreateCar(), you are doing three distinct things, contructing a car, aquiring an ofstream, and writing the value of a car to said ofstream. This creates a host of problems with an OO design. First, it is both a constructor disguised as a mutator and an accessor. You should make this two functions. A function:

printcar(ostream&);

Would solve this. Notice that the ostream is NOT hard coded into the function. It is also NOT a member of the class. This is another facet of OO programming, that which is not germane to objects of type Car (such as the file in which objects of type Car are stored) should not be a member of a Car.

So, a better main() might go along the lines of:

Code:

int main () {
Car database(cout, cin);
ofstream outfile("Car.dat", ios::out);
database.printcar(outfile);
return 0;
}

As for your odd constructor, your default constructor should set the values to SOMETHING. The lines:

year;
engine;

All they do is return the value of these undefined variables and throw them away. A default constructor should always leave an object in a predictable state, so you should assign all the unsigned short ints to 0 and all the strings to "" or some other placeholder.

Remember, in good OO a function does only one thing, is only given what is required for it to do that, but does not make any assumptions about what this will be. Hardcoding of constants is to be minimized or eliminated, and only those things logically germane to an object are members.

Hope this helps.

Additionally, hardcoding cin as the istream is not good OO programming.

can you explain that a little more. what's hardcoding?

No it's not a good example of good OOP.

this is a better example of OOP (not the best though...never liked examples featuring cars... hehe):

Code:

class IBase
{
// here you put code needed for databas handling (like writethis damncartoaDBfile or something...)
};

class IVehicle
{
//here you put code and variables common to all vehicles
};

class CCommonCar: public IVehicle, public IBase
{
// Here is code and vars all cars share
};

class CSwedishCar: public CCommonCar
{
// here is code all swedish cars share
};

class CSportCar: public CCommonCar
{
//sportcars code
};

class CPorche: public CSportCar
{
//porche specific code
};

class CPorcheX11TBirdOrWhatEverModel: public CPorche
{
//variables for this speciffic porche model (might not be any though)
};

class CVolvo: public CSwedishCar
{
// you know that i'm gonna write here
};

class CBuss: public IVehicle
{
//Code that all busses have in common
};

Now... you could break the car down to parts and make classes for that to. Like wheels and motor and all that kind of stuff... Things to note: the interface classes (Ibase, Ivehicle) should have virtual functions so that they can be overloaded. The fictive function IBase->StoreCar will look different for each car type, so overloading is necicare.

Well I could go on forever about good OOP, but I won't if you need help just send me an msg...

What exactly does OOP mean?
The only time i have heard of such a thing was in Utopia when it meant Out Of Protection

Object Oriented Programming

Yaaargh of course. I should have thought of that

What I mean by hardcoding is using a specific istream instread of accepting one.

For example:

Code:

int getsomeinteger() {
int i;
cin >> i;  //hardcoded
return i;
}

uses a hardcoded istream as cin. For a toy prog, that is often acceptable, but for a truly standalone object, you should be able to use an istream (istringstream, ifstream, wcin, etc)

So:

Code:

 
int getsomeinteger (istream& istr) {
int i;
istr >> i;  //not hard coded
return i;
}

could be provided as an overloaded function, which allows the class to be more of it's own object. OO writing seems needlessly complex at first, after all couldn't you just go in and chance cin to ifstream if you needed a file? But, what if the person using you Car class isn't you? What if someone needs to maintain or wrap your Car class? That is the advantage of OOP, as you could use your objects in a system that used input from a file, or some wierd hardware device, simple by changing how you call it, so you would not even need to know how a car worked to drive one. That often called implementation-hiding is another key prinicple of OOP.