Am I using classes correctly?

856 Views 18 Replies 6 Participants Last post by lordikon, Dec 10, 2009

godsgift2dagame

Discussion Starter · Dec 6, 2009

Code:

Code:

#include <iostream>
#include <string>
using namespace std;

class Vendor
{
    private:
            string name;
            int price;
            int quantity;
            int location;
            char letter;
            int startingQuantity;

     public:
            /*Vendor();
            Vendor(string,int,int,int,int);
            ~Vendor();*/
            void vendItem();
            int getQuantity(){return quantity;}
            string* getName();
            int* getPrice();
            void setAlphaLocation(char);
            char getAlphaLocation(){return (letter);}
            int getLocation(){return location;}
            int* getStartQuantity();
};

int* Vendor::getStartQuantity()
{
    int *start_quantity = new int[12];
    start_quantity[0] = 25;
    start_quantity[1] = 25;
    start_quantity[2] = 25;
    start_quantity[3] = 25;
    start_quantity[4] = 25;
    start_quantity[5] = 25;
    start_quantity[6] = 25;
    start_quantity[7] = 25;
    start_quantity[8] = 25;
    start_quantity[9] = 25;
    start_quantity[10] = 25;
    start_quantity[11] = 25;

    return start_quantity;

}

int* Vendor::getPrice()
{
    int *price = new int[12];
    price[0] = 25;
    price[1] = 25;
    price[2] = 25;
    price[3] = 25;
    price[4] = 25;
    price[5] = 25;
    price[6] = 25;
    price[7] = 25;
    price[8] = 25;
    price[9] = 25;
    price[10] = 25;
    price[11] = 25;

    return price;

}

string* Vendor::getName()
{
    //start here
    string *name = new string[12];
    name[0] = "A1. Mars Candy Bar";
    name[1] = "A2. Snickers Bar";
    name[2] = "A3. Twix Candy Bar";
    name[3] = "B1. Kit-Kat Bar";
    name[4] = "B2. Peanut Candy Bar";
    name[5] = "B3. Oreos Candy Bar";
    name[6] = "C1. Reeses's...";
    name[7] = "C2. Hershey's";
    name[8] = "C3. 3 Muske...";
    name[9] = "D1. Ferrero Rocher";
    name[10] = "D2. Butterf...";
    name[11] = "D3. Baby Ruth";

    return name;
}

int main()
{
    Vendor user;
    string* name = user.getName();
    int* price = user.getPrice();
    int* startingQuantity = user.getStartQuantity();
    char* letter;
    for(int i = 0; i < 12; i++)
    {
    cout << name[i] << " - " << price[i] << " cents\\" << "Currently available: " << startingQuantity[i] << endl;
    }
    cout << "Enter letter: ";
    cin >> letter;
    cout << "You chose " << user.getAlphaLocation(letter);

    getchar();
    getchar();
    return 0;
}

Hey guys, I need help determining if the way I've got my classes "set up" is legit practice or whether I need to re-do them.

Thanks!

See less See more

1 - 19 of 19 Posts

Spotswood

· #2 · Dec 7, 2009

Not knowing exactly what you are trying to model, its hard to say whether your Vendor class is "correct" or not. However, I would have expected all of the data associated with the class allocated in the class constructor, and not in the individual getXXX() methods.

godsgift2dagame

Discussion Starter · #3 · Dec 7, 2009

Thanks for the response!

I'm trying to make a vending machine, but I'm worried about whether it's appropriate to use cout's and those types of things inside member functions...

Spotswood

· #4 · Dec 7, 2009

A method/class should have only one purpose. Adding I/O to a vending machine class would break that rule.

error10

· #5 · Dec 7, 2009

2

Quote:

Originally Posted by Spotswood

View Post

A method/class should have only one purpose. Adding I/O to a vending machine class would break that rule.

Input coins, output Coke.

See less See more

Reactions: 1

Spotswood

· #6 · Dec 7, 2009

4

Quote:

Originally Posted by error10

View Post

Input coins, output Coke.

What I was thinking of was having an ICoinReader and an IKeypad interface which could then be implemented using cin and cout, rather than sprinkling uses of cin/cout throughout the code.

See less See more

Reactions: 1

dante020

· #7 · Dec 7, 2009

I'm not exactly sure why you are creating a new array on each call. You should probably create the arrays in the constructor and just return a member variable:

Code:

Code:

#include <iostream>
#include <string>
using namespace std;

class Vendor
{
    private:
            string *name;
            int *price;
            int *quantity;
            int *location;
            const int count = 12;
            char letter;
            int startingQuantity;

     public:
            /*Vendor();
            Vendor(string,int,int,int,int);
            ~Vendor();*/
            void vendItem();
            int getQuantity(){return quantity;}
            string* getName();
            int* getPrice();
            void setAlphaLocation(char);
            char getAlphaLocation(){return (letter);}
            int getLocation(){return location;}
            int* getStartQuantity();
};

Vendor::Vendor(...)
{
    quantity = new int[count];
    start_quantity[0] = 25;
    start_quantity[1] = 25;
    start_quantity[2] = 25;
    start_quantity[3] = 25;
    start_quantity[4] = 25;
    start_quantity[5] = 25;
    start_quantity[6] = 25;
    start_quantity[7] = 25;
    start_quantity[8] = 25;
    start_quantity[9] = 25;
    start_quantity[10] = 25;
    start_quantity[11] = 25;

    price = new int[count];
    price[0] = 25;
    price[1] = 25;
    price[2] = 25;
    price[3] = 25;
    price[4] = 25;
    price[5] = 25;
    price[6] = 25;
    price[7] = 25;
    price[8] = 25;
    price[9] = 25;
    price[10] = 25;
    price[11] = 25;

    //start here
    name = new string[count];
    name[0] = "A1. Mars Candy Bar";
    name[1] = "A2. Snickers Bar";
    name[2] = "A3. Twix Candy Bar";
    name[3] = "B1. Kit-Kat Bar";
    name[4] = "B2. Peanut Candy Bar";
    name[5] = "B3. Oreos Candy Bar";
    name[6] = "C1. Reeses's...";
    name[7] = "C2. Hershey's";
    name[8] = "C3. 3 Muske...";
    name[9] = "D1. Ferrero Rocher";
    name[10] = "D2. Butterf...";
    name[11] = "D3. Baby Ruth";

}

int* Vendor::getQuantity()
{
    return quantity; 
}

int* Vendor::getPrice()
{   
    return price; 
}

string* Vendor::getName()
{
    return name;
}

int Vendor::getCount()
{
    return count;
}

int main()
{
    Vendor user;
    char* letter;
    for(int i = 0; i < 12; i++)
    {
    cout << user.getName()[i] << " - " << user.getPrice()[i] << " cents\\" << "Currently available: " << user.getQuantity()[i] << endl;
    }
    cout << "Enter letter: ";
    cin >> letter;
    cout << "You chose " << user.getAlphaLocation(letter);

    getchar();
    getchar();
    return 0;
}

Not sure if it all works but its a step in the right direction.

See less See more

Reactions: 1

godsgift2dagame

Discussion Starter · #8 · Dec 8, 2009

First, this is my classrooms first time using classes and our professor stated we could use cin and cout's in the member functions. However, I really want to move away from that, if possible...

Second, thanks a bunch for the kind responses! I really expected to be attacked for not knowing their purpose entirely. Over the weekend, I went to sleep every MORNING at 5 A.M. because I was addicted to learning C++ and getting ahead of class.

P.S. I won't be using anything besides members inside the functions...so GO ME

+1 rep to all

See less See more

dante020

· #9 · Dec 8, 2009

2

Quote:

Originally Posted by godsgift2dagame

View Post

First, this is my classrooms first time using classes and our professor stated we could use cin and cout's in the member functions. However, I really want to move away from that, if possible...

Second, thanks a bunch for the kind responses! I really expected to be attacked for not knowing their purpose entirely. Over the weekend, I went to sleep every MORNING at 5 A.M. because I was addicted to learning C++ and getting ahead of class.

P.S. I won't be using anything besides members inside the functions...so GO ME

+1 rep to all

Keep at it. I'm in my senior year of a computer science degree and I don't regret it at all. Its a lot of programming but I'm always up for a challenge!

See less See more

godsgift2dagame

Discussion Starter · #10 · Dec 8, 2009

Quote:

Originally Posted by dante020

View Post

Keep at it. I'm in my senior year of a computer science degree and I don't regret it at all. Its a lot of programming but I'm always up for a challenge!

Sorry to bother, but did you see any serious issues with the code or am I going the right way?

See less See more

godsgift2dagame

Discussion Starter · #11 · Dec 8, 2009

Quote:

Originally Posted by dante020

View Post

I'm not exactly sure why you are creating a new array on each call. You should probably create the arrays in the constructor and just return a member variable:

Code:

Code:

#include <iostream>
#include <string>
using namespace std;

class Vendor
{
    private:
            string *name;
            int *price;
            int *quantity;
            int *location;
            const int count = 12;
            char letter;
            int startingQuantity;

     public:
            /*Vendor();
            Vendor(string,int,int,int,int);
            ~Vendor();*/
            void vendItem();
            int getQuantity(){return quantity;}
            string* getName();
            int* getPrice();
            void setAlphaLocation(char);
            char getAlphaLocation(){return (letter);}
            int getLocation(){return location;}
            int* getStartQuantity();
};

Not sure if it all works but its a step in the right direction.

I thought C++ forbids initialization of static members?

See less See more

godsgift2dagame

Discussion Starter · #12 · Dec 8, 2009

Also, how do I include TWO classes for one application?

I assume it can't be something like this:

Code:

Code:

class A 
{
blah blah

};

class B
{
blah blah

};

int main()
{

}

and probably will be

Code:

Code:

#include <"A">
#include <"B">

int main()
{

}

Is that correct?

See less See more

dante020

· #13 · Dec 8, 2009

Quote:

Originally Posted by godsgift2dagame

View Post

Also, how do I include TWO classes for one application?

I assume it can't be something like this:

Code:

Code:

class A 
{
blah blah

};

class B
{
blah blah

};

int main()
{

}

and probably will be

Code:

Code:

#include <"A">
#include <"B">

int main()
{

}

Is that correct?

You actually can do it like your first example, but generally, you want to break classes up into header and implementation files:

a.h

Code:

Code:

#ifndef A_H
#define A_H

#include <iostream>
#include <string>
using namespace std;

class A
{
    private:
    //member variables

    public:
    //methods (NOT implemented)
    A();
    void randomA();
};
#endif

a.cc

Code:

Code:

#include "a.h"

A::A()
{
    // stuff
}

void A::randomA()
{
    // stuff
}

main.cc

Code:

Code:

#include <iostream>
#include <string>

#include "a.h"

using namespace std;

int main()
{
    // stuff
    return 0;
}

Just add as many classes as you want in this fashion.

See less See more

godsgift2dagame

Discussion Starter · #14 · Dec 9, 2009

2

I'm getting the error that I have undefined references to class Vendor.

Here's my code after trying to emulate yours.

vendor.h

Code:

Code:

#ifndef Vendor_H
#define Vendor_H

#include <iostream>
#include <string>
using namespace std;

class Vendor
{
      private:
              int quantity;
              int numLoc;
              char Loc;
              int BegQuantity;
              string name;
              int price;
              int start_quantity;
      public:
              void setLocation(int);
              int getLocation(){return numLoc;}
              void setQuantity(int);
              int getQuantity(){return quantity;}
              void setCharLoc(char);
              char getCharLoc(){return Loc;}
              string* getName();
              int* getPrice();
              int* getStartQuantity();
};
#endif

That's the header file.

Down below is the implementation of the class Vendor.

vendor.cpp

Code:

Code:

#include "vendor.h"

string* Vendor::getName()
{
    string *name = new string[12];
    name[0] = "A1. Mars";
    name[1] = "A2. Snickers";
    name[2] = "A3. Twix";
    name[3] = "B1. Kit-Kat";
    name[4] = "B2. Peanut";
    name[5] = "B3. Oreos";
    name[6] = "C1. Reeses's...";
    name[7] = "C2. Hershey's";
    name[8] = "C3. 3 Muske...";
    name[9] = "D1. Ferrero";
    name[10] = "D2. Butterf...";
    name[11] = "D3. Baby Ruth";

    return name;
}

int* Vendor::getPrice()
{
    int *price = new int[12];
    price[0] = 90;
    price[1] = 75;
    price[2] = 55;
    price[3] = 70;
    price[4] = 85;
    price[5] = 75;
    price[6] = 60;
    price[7] = 115;
    price[8] = 100;
    price[9] = 125;
    price[10] = 95;
    price[11] = 150;

    return price;    
}

int* Vendor::getStartQuantity()
{
    int *start_quantity = new int[12];
    start_quantity[0] = 25;
    start_quantity[1] = 25;
    start_quantity[2] = 25;
    start_quantity[3] = 25;
    start_quantity[4] = 25;
    start_quantity[5] = 25;
    start_quantity[6] = 25;
    start_quantity[7] = 25;
    start_quantity[8] = 25;
    start_quantity[9] = 25;
    start_quantity[10] = 25;
    start_quantity[11] = 25;

    return start_quantity; 
}

void Vendor::setCharLoc(char letter)
{
     Loc = letter;
}

void Vendor::setLocation(int location)
{
     numLoc = location;
}

void Vendor::setQuantity(int amount)
{
    quantity = amount;
}

And finally, my Vending Machine as a whole...

Vending-Machine.cpp

Code:

Code:

#include <iostream>
#include <string>

#include "vendor.h"

using namespace std;

int main()
{
    Vendor user;

    //Variables
    char Loc;
    int NumLoc;
    int quantity;
    int total = 0;
    int quantityRemaining;

    //DISPLAY ITEMS
    string* names = user.getName();
    int* price = user.getPrice();
    int* start_quantity = user.getStartQuantity();
    int* BegQuantity = user.getStartQuantity();
    for(int i = 0; i < 12; i++)
    {
            cout << names[i] << "\\" << price[i] << "\currently available: " << BegQuantity[i] << endl;
    }

    cout << "Enter letter of item: ";
    cin >> Loc;
    while(Loc)
    {
         if(isdigit(Loc))
         {
         cout << "Please enter a letter...: ";
         cin >> Loc;
         continue;
         }

         else 
         break;
    }

    cout << "Enter number of item: ";
    cin >> NumLoc;

    cout << "How many: ";
    cin >> quantity;

    for(int i = 0; i < 12; i++)
    {
            while(quantity)
            {
                if(quantity > BegQuantity[i])
                {
                cout << "NOT ENOUGH AVAILABLE!  Try again: ";
                cin >> quantity;
                continue;
                }

                else
                break;
            }
    }

    if(Loc == 'A' && NumLoc == 1 || Loc == 'a' && NumLoc == 1)
    {
           total = price[0] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[0] - quantity;
           cout << "Quantity remaining of " << names[0] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'A' && NumLoc == 2 || Loc == 'a' && NumLoc == 2)
    {
           total = price[1] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[1] - quantity;
           cout << "Quantity remaining of " << names[1] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'A' && NumLoc == 3 || Loc == 'a' && NumLoc == 3)
    {
           total = price[2] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[2] - quantity;
           cout << "Quantity remaining of " << names[2] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'B' && NumLoc == 1 || Loc == 'b' && NumLoc == 1)
    {
           total = price[3] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[3] - quantity;
           cout << "Quantity remaining of " << names[3] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'B' && NumLoc == 2 || Loc == 'b' && NumLoc == 2)
    {
           total = price[4] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[4] - quantity;
           cout << "Quantity remaining of " << names[4] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'B' && NumLoc == 3 || Loc == 'b' && NumLoc == 3)
    {
           total = price[5] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[5] - quantity;
           cout << "Quantity remaining of " << names[5] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'C' && NumLoc == 1 || Loc == 'c' && NumLoc == 1)
    {
           total = price[6] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[6] - quantity;
           cout << "Quantity remaining of " << names[6] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'C' && NumLoc == 2 || Loc == 'c' && NumLoc == 2)
    {
           total = price[7] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[7] - quantity;
           cout << "Quantity remaining of " << names[7] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'C' && NumLoc == 3 || Loc == 'c' && NumLoc == 3)
    {
           total = price[8] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[8] - quantity;
           cout << "Quantity remaining of " << names[8] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'D' && NumLoc == 1 || Loc == 'd' && NumLoc == 1)
    {
           total = price[9] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[9] - quantity;
           cout << "Quantity remaining of " << names[9] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'D' && NumLoc == 2 || Loc == 'd' && NumLoc == 2)
    {
           total = price[10] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[10] - quantity;
           cout << "Quantity remaining of " << names[10] 
                << ": " << quantityRemaining;
    }

    if(Loc == 'D' && NumLoc == 3 || Loc == 'd' && NumLoc == 3)
    {
           total = price[11] * quantity;
           cout << "The total is: " << total << endl;
           quantityRemaining = BegQuantity[11] - quantity;
           cout << "Quantity remaining of " << names[11] 
                << ": " << quantityRemaining;
    }

    getchar();
    getchar();
    return 0;
}

See less See more

lordikon

· #15 · Dec 9, 2009

I'm still not seeing a constructor. Initializing your members at the top of each function is a little scary, but I guess it will work.

Also, rather than use arrays with hard-coded sizes and having to pass back pointers to the first element in that array, you could just use std::vector and then pass it in and out of the function by reference.

So this:
int* doStuff()
{
int* arrayOfStuff = new int[2];
arrayOfStuff[0] = 5;
arrayOfStuff[1] = 1;
}

Becomes this:
void doStuff(std::vector<int>& myStuff)
{
myStuff.push_back(5);
myStuff.push_back(1);
}

By doing the above, you can change the size of your "array" easily. Right now you're using '12'. This means that any place that uses that function, magically has to know that the int* it is getting is pointing to an int array of 12 elements.

Also, you're initializing these arrays, but I see no delete to remove them. Your program is causing memory leaks. Anytime you use the 'new' keyword, you must call 'delete' on that data somewhere. In your case, 'delete[]'.

See less See more

Reactions: 1

pastadiablo

· #16 · Dec 9, 2009

3

Quote:

Originally Posted by godsgift2dagame

View Post

did you see any serious issues with the code or am I going the right way?

So as everyone keeps mentioning, you really shouldn't be creating a brand new array every time you call those Get functions - you should create the data in the constructor. But, seeing as a bunch of people have already said this without a response from you, I'm wondering if you know what a constructor is?

The reason you shouldn't put them all into each get call is because that means that every time you want to access that information with a get call your program allocates a ton of new memory, stuffs information into it, and then reads that. Unfortunately that means that if you make a lot of get calls you have a ton of different pockets in your memory that all store the same exact information. Its not efficient! Whereas if you just declare this information one time (in the constructor when the instance of the class is first instantiated) then you only have ONE spot in memory with all of this data stored.

Quote:

Originally Posted by godsgift2dagame

View Post

I thought C++ forbids initialization of static members?

It does, unless those members are constant (i.e. they never change) and are declared so, such as with the const keyword.

Quote:

Originally Posted by godsgift2dagame

View Post

I'm getting the error that I have undefined references to class Vendor.

Which files are the undefined references showing up in? vendor.cpp or Vending-Machine.cpp, or both?

See less See more

pastadiablo

· #17 · Dec 9, 2009

Quote:

Originally Posted by lordikon

View Post

I'm still not seeing a constructor. Initializing your members at the top of each function is a little scary, but I guess it will work.

Also, rather than use arrays with hard-coded sizes and having to pass back pointers to the first element in that array, you could just use std::vector and then pass it in and out of the function by reference.

So this:
int* doStuff()
{
int* arrayOfStuff = new int[2];
arrayOfStuff[0] = 5;
arrayOfStuff[1] = 1;
}

Becomes this:
void doStuff(std::vector<int>& myStuff)
{
myStuff.push_back(5);
myStuff.push_back(1);
}

By doing the above, you can change the size of your "array" easily. Right now you're using '12'. This means that any place that uses that function, magically has to know that the int* it is getting is pointing to an int array of 12 elements.

Also, you're initializing these arrays, but I see no delete to remove them. Your program is causing memory leaks. Anytime you use the 'new' keyword, you must call 'delete' on that data somewhere. In your case, 'delete[]'.

^-This. All of it.

See less See more

Reactions: 1

godsgift2dagame

Discussion Starter · #18 · Dec 10, 2009

Hey guys,

Thank you sooo much for the help!

First, I'd like to briefly address a comment the new guy, pastadiablo, made. He questioned whether I knew what a constructor was...I didn't. I don't want to make it seem like I ignored it because I didn't read it...I did...a couple times. But I was scared of admitting I didn't know of such a thing. I believe I do know, after having read the cplusplus*com version of it.

Regarding lordikon's suggestion of using a vector type to create these dynamic arrays, I'm afraid I suck at it. Real Bad. I'll try to get it to work tonight.

Pasta, I'll give you the referenced files that aren't working either tonight or tomorrow.

Again, thanks a bunch guys and +1 rep!

lordikon

· #19 · Dec 10, 2009

Quote:

Originally Posted by godsgift2dagame

View Post

Hey guys,

Thank you sooo much for the help!

First, I'd like to briefly address a comment the new guy, pastadiablo, made. He questioned whether I knew what a constructor was...I didn't. I don't want to make it seem like I ignored it because I didn't read it...I did...a couple times. But I was scared of admitting I didn't know of such a thing. I believe I do know, after having read the cplusplus*com version of it.

Regarding lordikon's suggestion of using a vector type to create these dynamic arrays, I'm afraid I suck at it. Real Bad. I'll try to get it to work tonight.

Pasta, I'll give you the referenced files that aren't working either tonight or tomorrow.

Again, thanks a bunch guys and +1 rep!

Here's a super quick example of using a vector. I'm writing this here, so I can't guarantee it'll even compile.

Code:

Code:

#include <vector>

int main()
{
   std::vector<int> myInts;
   myInts.push_back(1);   // This will push the value 1 onto the end of the vector

   // You can treat a vector very very much like you would an array
   myInts[0] = 2;   // Now index 0 has '2' in it instead of the '1' we pushed back

   // Want to make sure you don't go outside of the bounds of the vector?
   const int indexToCheck = 2;
   if ( indexToCheck > (myInts.size() - 1) )
   {
      // indexToCheck is within bounds
   }
   else
   {
      // indexToCheck is out of the bounds of myInts
   }

   // Clear the entire vector
   myInts.clear();

   // Exit program
   return 0;
}

See less See more

1 - 19 of 19 Posts

This is an older thread, you may not receive a response, and could be reviving an old thread. Please consider creating a new thread.