Messy bunch o' code

This is a discussion on Messy bunch o' code within the C++ Programming forums, part of the General Programming Boards category; Well, I made this simple program, which is not complete yet, but any suggestions of improving code would be great ...

  1. #1
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717

    Messy bunch o' code

    Well, I made this simple program, which is not complete yet, but any suggestions of improving code would be great
    Anyways, I'm mostly posting here 'cause I need to transfer the code, now that I don't have access to my home PC :/ oh and I do know that the code should be seperated into files, but I'm too lazy to do that :P

    main.cpp
    Code:
    #include <iostream>
    #include "product.h"
    
    using std::cout;	using std::endl;
    using std::vector;	using std::string;
    using std::cin;
    
    void addProd( const string name, const double price, const categories category,
    			 vector<product>& vec);
    void addGatherInfo(vector<product>& prodVec);
    string categoryToString(const categories& check);
    void prodRemove(string name, vector<product>& vec);
    
    int main(){
    	bool run = true;
    	vector<product> prodVec;
    	while(run == true){
    
    	string input;
    	cout<< "\nWant to add a new product? Type: add" << endl;
    	cout<< "Want to see a list of added products? Type: list" << endl;
    	cout<< "Want to remove a product? Type: remove" << endl;
    	cout<< "Want to exit the program? Type: exit" << endl;
    	cin>> input;
    	if(input == "add"){
    		addGatherInfo(prodVec);
    	}
    
    	if(input == "list"){
    		for(vector<product>::size_type i = 0; i != prodVec.size(); ++i){
    
    			cout<< "\nProduct name: " << prodVec[i].name;
    			cout<< "\nProduct price: " << prodVec[i].price;
    			cout<< "\nProduct category: " << categoryToString(prodVec[i].category);
    			cout<< "\n----------------\n";
    		}
    	}
    
    	if(input == "remove"){
    		string name;
    		cout<< "Please type in a name for the product: ";
    		cin>>name;
    		prodRemove(name, prodVec);
    	}
    
    	
    
    	if(input == "exit"){
    		run = false;
    	}	
    	
    	}
    	return 0;
    }
    
    void addProd( const string name, const double price, const categories category,
    						vector<product>& vec) {
    
    				product prod;
    				prod.name = name;
    				prod.price = price;
    				prod.category = category;
    
    				vec.push_back(prod);
    
    }
    
    void addGatherInfo(vector<product>& inpVec){
    
    		string name;
    		double price;
    		categories category;
    
    		cout<< "Please type in a name for the product: ";
    		cin>> name;
    		cout<< "\nPlease type in a price for the product: ";
    		cin>> price;
    		cout<< "\nPlease choose a category for the product: " <<endl;
    		cout<< "1. vegetables\n" << "2. drink\n" << "3. meat\n";
    		cout<< "Type in one of the numbers for those categories: ";
    		int choNum;
    		cin>> choNum;
    		switch(choNum){
    			case 1:
    				category.vegetable = true;
    				break;
    			case 2:
    				category.drink = true;
    				break;
    			case 3:
    				category.meat = true;
    				break;
    		}
    	
    		addProd(name, price, category, inpVec);
    }
    
    string categoryToString(const categories& check){
    	if(check.vegetable == true)
    		return "Vegetable";
    	if(check.drink == true)
    		return "Drink";
    	if(check.meat == true)
    		return "Meat";
    
    	return "Product category not defined!";
    }
    
    void prodRemove(string name, vector<product>& vec){
    	for(vector<product>::size_type i = 0; i != vec.size(); ++i){
    		if(vec[i].name == name){
    			vec.erase(vec.begin() + i);
    			break;
    		}
    	}
    }
    main.h
    Code:
    #ifndef MAIN_H
    #define MAIN_H
    
    #include <string>
    #include <vector>
    
    struct categories{
    	bool vegetable;
    	bool drink;
    	bool meat;
    };
    
    /*struct product {
    	std::string name;
    	double price;
    	categories category;
    };*/
    
    #endif
    product.cpp
    Code:
    #include "product.h"
    
    using std::string;
    
    product::product(){
    	category.vegetable = false;
    	category.drink = false;
    	category.meat = false;
    }
    
    product::~product(){
    }
    product.h
    Code:
    #ifndef PRODUCT_H
    #define PRODUCT_H
    
    #include "main.h"
    
    class product
    {
    public:
    
    	product();
    	~product();
    
    	std::string name;
    	double price;
    	categories category;
    
    protected:
    
    
    };
    
    #endif
    Last edited by Akkernight; 01-23-2009 at 03:18 AM. Reason: Update
    Currently research OpenGL

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
    struct categories{
    	bool vegetable;
    	bool drink;
    	bool meat;
    };
    If you never want vegetable, drink and meat to be set at the same time, then an enum is probably a better choice. [Your current output doesn't appear to deal with more than one set right now anyways].

    Code:
    	if(input == "/add"){
    (And the others like it): Why do you have a slash at the front of the name of the command - seems completely arbitrary and unnecessary.

    --
    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.

  3. #3
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717
    It's just what I'm used to seeing from games(And looks more clean by my eyes) :P and I've never learnt about enum :/
    Currently research OpenGL

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Sorry, hit submit before I meant to, so posting a second post.

    Code:
    vector<product> addProd( const string name, const double price, const categories category,
    						vector<product>& vec) {
    
    				product prod;
    				prod.name = name;
    				prod.price = price;
    				prod.category = category;
    
    				vec.push_back(prod);
    
    				return vec;
    }
    Why pass in a reference to the vector, and then return it. The latter copies the entire vector [and as you don't use the returned value at all, it is a complete waste of time].

    Code:
    		for(vector<product>::size_type i = 0; i != prodVec.size(); ++i){
    
    			cout<< "\nProduct name: " << prodVec[i].name;
    			cout<< "\nProduct price: " << prodVec[i].price;
    			cout<< "\nProduct category: " << checkCategory(prodVec[i].category);
    			cout<< "\n----------------\n";
    		}
    This should probably be a function. The name "checkCategory" would be better called "categoryToString" or something like that.

    --
    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
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717
    Well the reason I used referance is 'cause I thought that was the right thing for that use xD Thought I was being clever 8-)
    Currently research OpenGL

  6. #6
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Akkernight View Post
    Well the reason I used referance is 'cause I thought that was the right thing for that use xD Thought I was being clever 8-)
    Yes, that IS clever. Returning a copy of the vector is not.

    --
    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.

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Akkernight View Post
    It's just what I'm used to seeing from games(And looks more clean by my eyes) :P and I've never learnt about enum :/
    Time to start learning about enums then.

    Using a "simple" user-interface is a good idea. Adding extra characters for some arbitrary reason is not good user-interface design.

    --
    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.

  8. #8
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717
    Oh yeah! I'm calling main() in main(), making it recursive, this was just something I did, 'cause I had no other idea on how to not shut the program each time it ran, but is this wise to do?
    Currently research OpenGL

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Akkernight View Post
    Oh yeah! I'm calling main() in main(), making it recursive, this was just something I did, 'cause I had no other idea on how to not shut the program each time it ran, but is this wise to do?
    Absolutely NOT the right thing to do. I missed that. Just use some form of loop (for, while, do-while) - since the call to main() is unconditional, just doing an while(true) or for(; will do what you want.

    --
    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 hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,799
    Code:
    void prodRemove(string name){
    	for(vector<product>::size_type i = 0; i != prodVec.size(); ++i){
    		if(prodVec[i].name == name){
    			prodVec.erase(prodVec.begin() + i);
    			break;
    		}
    	}
    }
    From the other post related to the prodRemove function you should be using < and not !=.

    Also, you should avoid using globals as much as possible so you should probably be passing in a vector to the function (you are passing in one to other functions [addGatherInfo/addProd] but not this one for some reason).
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  11. #11
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717
    Well, I didn't make addProd vector thing a reference and it didn't work anymore :P

    And I told you I fixed it xD Just by making the vector global :P I'll fix that...
    Currently research OpenGL

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Akkernight View Post
    Well, I didn't make addProd vector thing a reference and it didn't work anymore :P

    And I told you I fixed it xD Just by making the vector global :P I'll fix that...
    No, no, no! Do not EVER fix a problem by making a variable global [Ok, so once in a while that may well be the right thing, but that is VERY rare].

    You should use a reference in addProd [a reference means that you pass the original vector rather than a copy, so it's exactly the right thing to do here, since you want to change the content of the vector].

    It is the "return vec" that is wrong - make it a void function, and simply do not return anything, and all will be fine.

    --
    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.

  13. #13
    Hail to the king, baby. Akkernight's Avatar
    Join Date
    Oct 2008
    Location
    Faroe Islands
    Posts
    717
    Ohh... I see xD Thanks!

    Next thing will be files... That was the ostream library, right?
    Last edited by Akkernight; 01-22-2009 at 07:51 AM.
    Currently research OpenGL

  14. #14
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,662
    Speaking of call by reference, you should be passing references to string and the other struct as well. Making the price parameter const is only useful to you as the implementer of the function. It makes no difference to the caller whether price is const or non-const since the argument is passed by value.

    Later on should product become a full blown class addProd would then be a member 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

  15. #15
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by laserlight View Post
    Later on should product become a full blown class addProd would then be a member function.

    Surely you mean that addProd would be the member of an object that holds the vector of products?

    --
    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.

Page 1 of 3 123 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Proposal: Code colouring
    By Perspective in forum A Brief History of Cprogramming.com
    Replies: 28
    Last Post: 05-14-2007, 07:23 AM
  2. Values changing without reason?
    By subtled in forum C Programming
    Replies: 2
    Last Post: 04-19-2007, 10:20 AM
  3. Updated sound engine code
    By VirtualAce in forum Game Programming
    Replies: 8
    Last Post: 11-18-2004, 11:38 AM
  4. True ASM vs. Fake ASM ????
    By DavidP in forum A Brief History of Cprogramming.com
    Replies: 7
    Last Post: 04-02-2003, 03:28 AM
  5. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM

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