Thread: Suggestions on how to tweak my code

  1. #1
    Registered User
    Join Date
    Feb 2013
    Posts
    24

    Suggestions on how to tweak my code

    In my code I'm creating a pricing system consisting of computer parts, where the user can enter in an item, the program will find it stored in a map, and return its price. The different categories the user can choose from are: the processor (cpu), RAM, cd drive, hard drive, modem, network card, wireless router (all which are stored in maps), and the shipping and base price to build the computer.

    The main issue I'm having with creating this is trying to reduce the number of functions, I have in my project. In my function below, "get_CPUprice", checks if a CPU processor is contained in the CPU map and if so returns its price, but I also have 6 other functions identical to this one that searches through the other maps I've created. I'm trying to figure out a way to get the prices for each of my options that are also in my maps in one function but I'm having difficulty coming up with a way.


    Below is a sample of my code.If someone needs me to clarify on
    something or would like to see the rest of my code please let me know.

    Code:
    //maps that store each item found during the search
    public:
     
         map<string,double> CPU;
         map<string,double> RAM;   
       map<string,double> HDdrive;
          map<string,double> CDdrive;
             map<string,double> NWcard;
                map<string,double> WLrouter;
                   map<string,double> Modem;
                     
         map<string,double>::iterator it; 
          
        CPUTester(){
        //Stores the CPU Processors
       CPU["AMD-Sempron-140"] =36.00;      
       CPU["AMD-PhenomII-X2560"] = 89.00; 
      CPU["Intel-Pentium-G2100T"] = 75.00;
       CPU["AMD-Opteron-6386SE"] =  1392.00;
       //Stores the memory    
        RAM["2GB"] = 40.00;
        RAM["4GB"] = 80.00;
         RAM["6GB"] = 120.00;
         
           //Hardrive sizes
      HDdrive["40GB"] = 8.00;
        HDdrive["160GB"] = 28.00;
         HDdrive["320GB"] = 45.00;
        HDdrive["1TB"] = 100.00; 
         HDdrive["2TB"] = 120.00;
        
     //Stores a CD Drive
         CDdrive["CD-ROM"] = 20.00;
        CDdrive["CD-RW"] = 35.00;
         CDdrive["DVD"]= 50.00;
         //modems
         Modem["Linksys-DOC3.0"] = 85.00;
         Modem["Netgear-N600"] = 115.00;
         Modem["Actiontec-DSL"] = 60.00;
       //network adapter cards
        NWcard["StarTech1-ST100S"] = 7.00;
        NWcard["Intellinet-150N"] = 10.00;
        NWcard["Wintec-FileMate"]=14.00;
        //wireless networks
           WLrouter["Asus-N66U"]=155.00;
              WLrouter["DLink-655"]=70.00;
        }
    Note: my other getter functions (RAM, cd drive, hard drive, modem, network card, and wireless router) use the same idea except with different map names and variables.

    Code:
      /* 
         @details - Checks if a processor is 
         * inside of CPU map
         * @return - Returns price, or error message 
         * 
         */
       void get_CPUprice(string processor){
           it = CPU.find(processor);
           if (it != CPU.end())
    
         cpu_price = it-> second;
         cout << "price: $" << cpu_price << endl;
    }else{
       cout << "Sorry," << it->first << 
               "not found" <<endl;
    }
        }
    Last edited by FloatingButter; 02-25-2013 at 04:09 PM.

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    What about something like

    Code:
         map<string,map<string,double>> Items;
    ...
       Items["CPU"]["AMD-Sempron-140"] =36.00;
    ...
        double getPrice ( std::string what, string item ) {  // eg, "CPU", "AMD-Opteron-6386SE"
            it = Items[what].find(item);
        }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    Alright, I'm gonna try this and see how I can modify the rest of my code. By the way, I forgot to mention that if I had gotten rid of the hypens in some of my map names and included spaces in between each word, when I try to type in a map name for some reason it won't return the price name even if I type it in exactly as it is shown in the map. Ex. "AMD Sempron 140" instead of "AMD-Sempron-140".I'm not sure why maps do not reconigze spaces in between words.

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > I'm not sure why maps do not reconigze spaces in between words.
    Perhaps it's more to do with your input function not capturing spaces to begin with.

    Remember cin >> var; will remove whitespace.

    Use getline() to read a whole line, including all significant spaces.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    Quote Originally Posted by Salem View Post
    > I'm not sure why maps do not reconigze spaces in between words.
    Perhaps it's more to do with your input function not capturing spaces to begin with.

    Remember cin >> var; will remove whitespace.

    Use getline() to read a whole line, including all significant spaces.
    When I tried to use this function my program would not print out a price, but would jump to the next output statement I had. So I've changed my code a bit, instead of the user having to type in each item, I've decided to: (1) show the user a list of each category found in the map,[see "showCategories"] and prompt them to pick one(2), then show them each item from that category (3)and after they select an item return the price. The problem I'm having with is figuring out how to return certain keys (not value)from a map if found.

    See code below.
    Code:
      void showCategories(){
            cout << "(1) CPU" << "\n";
            cout << "(2) RAM" << "\n";
           //.... 
        cout << "(7) Router" << "\n";
        }

    Code:
    void showItems(int number){
          
               if(number == 1){
               //show user items in CPU
                   //category
                   
            }else if (number == 2){
             //show user items in RAM category
            } 
    //...etc
    }

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    mixing cin >> var and getline() lead to their own set of complications.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User rogster001's Avatar
    Join Date
    Aug 2006
    Location
    Liverpool UK
    Posts
    1,472
    just curious why you indent like this -are you trying to show these data as somehow related? might get yelled at here haha - but.. I kind of like it, bit weird apart from lack of consistency - but then i think there is a consistency - looks kind if retro I think you should introduce a newline at each 'backspaced line' - To me i can see those lines as 'introducing' a new set of variables - Lots of ways to do that in layout of course, but i think thats a neat look but wouldnt scale to too many lines

    Code:
    //maps that store each item found during the search
    public:
     
         map<string,double> CPU;
         map<string,double> RAM;   
       map<string,double> HDdrive;
          map<string,double> CDdrive;
             map<string,double> NWcard;
                map<string,double> WLrouter;
                   map<string,double> Modem;
                     
         map<string,double>::iterator it;
    Last edited by rogster001; 02-26-2013 at 04:20 PM.
    Thought for the day:
    "Are you sure your sanity chip is fully screwed in sir?" (Kryten)
    FLTK: "The most fun you can have with your clothes on."

    Stroustrup:
    "If I had thought of it and had some marketing sense every computer and just about any gadget would have had a little 'C++ Inside' sticker on it'"

  8. #8
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    I actually just copied and pasted those, since i was lazy at the time and did not want to type out everything again.

  9. #9
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    Quote Originally Posted by FloatingButter View Post

    Code:
    //Shows the user each option to choose from.
      void showCategories(){
            cout << "(1) CPU" << "\n";
            cout << "(2) RAM" << "\n";
           //.... 
        cout << "(7) Router" << "\n";
        }

    Code:
    //Asks the user to enter in a number (representing the
    //category choosen) and shows the user items.
    void showItems(int number){
          
               if(number == 1){
               //show user items in CPU
                   //category
                   
            }else if (number == 2){
             //show user items in RAM category
            } 
    //...etc
    }
    I forgot to put comments in my code.

    The problem I'm having is figuring out a way to print out just a certain amount of items from the map (Ex. if the user enters a 1 for the CPU category, they would be shown the "AMD Sempron 140 , Intel Pentium G2130,and AMD Opteron 6386SE" value from the Items map)

  10. #10
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Mmm, is this giving it away?
    Code:
    #include<iostream>
    #include<string>
    #include<limits>
    #include<map>
    using namespace std;
    int main()
    {
      map<string,map<string,double> > Items;
      Items["CPU"]["AMD Sempron 140"] = 36.00;
      Items["CPU"]["AMD PhenomII X2560"] = 89.00;
      Items["CPU"]["Intel Pentium G2100T"] = 75.00;
      Items["CPU"]["AMD Opteron 6386SE"] =  1392.00;
      Items["RAM"]["2GB"] = 40.00;
      Items["RAM"]["4GB"] = 80.00;
      Items["RAM"]["6GB"] = 120.00;
      Items["HDdrive"]["40GB"] = 8.00;
      Items["HDdrive"]["160GB"] = 28.00;
      Items["HDdrive"]["320GB"] = 45.00;
      Items["HDdrive"]["1TB"] = 100.00;
      Items["HDdrive"]["2TB"] = 120.00;
      Items["CDdrive"]["CD ROM"] = 20.00;
      Items["CDdrive"]["CD RW"] = 35.00;
      Items["CDdrive"]["DVD"]= 50.00;
      Items["Modem"]["Linksys DOC3.0"] = 85.00;
      Items["Modem"]["Netgear N600"] = 115.00;
      Items["Modem"]["Actiontec DSL"] = 60.00;
      Items["NWcard"]["StarTech1 ST100S"] = 7.00;
      Items["NWcard"]["Intellinet 150N"] = 10.00;
      Items["NWcard"]["Wintec FileMate"]=14.00;
    
      string part;
      string item;
      cout << "Enter part > ";
      getline(cin,part);
      cout << "Enter item > ";
      getline(cin,item);
      
      map<string,map<string,double> >::iterator it;
      it = Items.find(part);
      if ( it != Items.end() ) {
        map<string,double>::iterator it2;
        it2 = it->second.find(item);
        if ( it2 != it->second.end() ) {
          cout << "Part " << part << " Item " << item << " costs " << it2->second << endl;
        } else {
          cout << "Item " << item << " doesn't exist" << endl;
        }
      } else {
        cout << "Part " << part << " doesn't exist" << endl;
      }
    }
    
    $ ./a.out 
    Enter part > CPU
    Enter item > AMD Sempron 140
    Part CPU Item AMD Sempron 140 costs 36
    $ ./a.out 
    Enter part > barney
    Enter item > rubble
    Part barney doesn't exist
    $ ./a.out 
    Enter part > CPU
    Enter item > super megatron
    Item super megatron doesn't exist
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  11. #11
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Better yet, load the parts list from an external file e.g. text file in ini format, rather than hard coding everything.
    e.g.:
    Code:
    [CPU]
    AMD Sempron 140=36.00
    AMD PhenomII X2560=89.00
    [RAM]
    2GB=40.00
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  12. #12
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    Quote Originally Posted by Salem View Post
    Mmm, is this giving it away?
    No this is fine, I've just modified it a bit cause I want the user to enter in a number for both the category and item instead of having to type in everything word for word. I'm also trying to get my program to get the total price of item after its gotten all of the prices together. I'd rather not have a function that takes "x" amount of parameters,since I have so many prices, so I've decided to have my function, getTotal, take no parameters and get the total from adding private variables. The issue I'm having is after the price is found in ,getPrice, I want the price to be stored in another function, storeItemPrice, so that my getTotal function can work correctly. I get this error:
    Code:
    "main.cpp:103:29: error: cannot convert 'std::map<int, double>' to 'double' in initialization"

    when I initialize this in getItemPrice:
    Code:
    double itemprice = it->second;
         storeItemPrice(category,itemprice);
    Here's my code:

    Code:
    #include<iostream>
    #include<string>
    #include<limits>
    #include<map>
    using namespace std;
    
    class CPUCatalog{
        private: 
        double cpu_price,RAM_price,CDD_price,
                HD_price,modem_price,router_price,
                nwcard_price,lapsize_price; 
        
        
        
    public:
        //create constructor 
        map<int,map<int,double> > Items;
        
       CPUCatalog(){
               //Stores the CPU Processors
       Items[1][1] =36.00;//AMD Sempron 140      
     Items[1][2] = 75.00;//Intel Pentium G2100T
       //Stores the memory    
       Items[2][1] = 40.00;//2GB
         Items[2][2] = 120.00;//6GB
           //Hardrive sizes
      Items[3][1] = 8.00;//40GB
        Items[3][2] = 100.00; //1TB
      
     //Stores a CD Drive
         Items[4][1] = 20.00;//CD-ROM
        Items[4][2] = 35.00;//CD-RW
         Items[4][3]= 50.00;//DVD
         //modems
         Items[5][1] = 85.00;//Linksys 3.0
         Items[5][2] = 115.00;//Netgear N600
       //network adapter cards
         Items[6][1] = 10.00;//Intellinet 150N
         Items[6][2]=14.00;//Wintec PCI
        //wireless routers
         Items[7][1] = 155.00;//Asus N66U
         Items[7][2] = 70.00;//D Link 655
         //laptop sizes
           Items[8][1]= 40.00; //14'
           Items[8][2]= 70.00;//16'
        
            
        }
        void showCategories(){
            cout << "CPU item categories:"  <<"\n";
            cout << "(1) CPU" << "\n";
            cout << "(2) RAM" << "\n";
            cout << "(3) CD drive" << "\n";
            cout << "(4) Hard drive" << "\n";
            cout << "(5) Modem" << "\n";
            cout << "(6) Network card" << "\n";
            cout << "(7) Wireless router" << "\n";
            cout << "(8) Laptop size" << "\n";
        }
          
        
           /*
           Stores the prices for each item found in getItemPrice.
           */
          void storeItemPrice(int category,int item){
              
                  if(category == 1){
               cpu_price =item;
              
            }else if (category == 2){
            RAM_price = item;
            } else if(category == 3){
                CDD_price = item;
                
            }else if (category == 4){
                
                HD_price = item;
            }else if (category == 5){
               modem_price = item;
            }else if (category ==6){
               router_price = item;
            }else if (category == 7){     
                nwcard_price = item;
            }else if(category == 8){
                lapsize_price = item;
            }
            
            
            
          }
        
        
        //create getPrice 
        void getItemPrice(int category,int item){
            
      map<int,map<int,double> >::iterator it;
      it = Items.find(category);
      if ( it != Items.end() ) {
        map<int,double>::iterator it2;
        it2 = it->second.find(item);
        if ( it2 != it->second.end() ) {
          cout << "Item costs: " << it2->second << endl;//double
         double itemprice = it->second;
         storeItemPrice(category,itemprice);
        } else {
          cout << "Item "  << " doesn't exist" << endl;
        }
      } else {
        cout << "category " << " doesn't exist" << endl;
      }
            
       
            
        }
        
       
            /*
        Uses private variables initialized from the "getPrice" function
        * to get the total.
        */
         void getTotal(){
            double total = 
            cpu_price + RAM_price +
            CDD_price + HD_price + modem_price +
            router_price + nwcard_price;
            cout << "total cost: $" << total << endl;
        }
        
        
        
        
        
        
          
          
    };
    
    int main()
    {
        
        CPUCatalog c; 
      
     int item;
      int category;
      
       cout << "Enter in CPU category number: ";
      cin >> category;
       cout << "Choose an item: (1) AMD Sempron 140 or "
              << "(2) Intel Pentium G2100T: " << endl;
      cin >> item;
      c.getItemPrice(category,item);
      
      
       cout << "Enter in RAM category number: ";
      cin >> category;
      cout << "Choose an item: (1) 2GB or (2) 6GB: " << endl;
      cin >> item;
      c.getItemPrice(category,item);
      
      
       cout << "Enter in hard drive category number: ";
      cin >> category;
      cout << "Choose an item: (1) 40GB or (2)160GB" << endl;
      cin >> item;
        c.getItemPrice(category,item);
      
      
        cout << "Enter in CD Drive category number: ";
      cin >> category;
      cout << "Choose an item: (1) CD-Rom (2) CD-RW, or (3) DVD" << endl;
      cin >> item;
      c.getItemPrice(category,item);
    
      
        cout << "Enter in modem category number: ";
      cin >> category;
      cout << "Choose an item: (1) Linksys 3.0 or (2)Netgear N600" << endl;
      cin >> item;
      c.getItemPrice(category,item);
    
      
        cout << "Enter in network card category number: ";
      cin >> category;
      cout << "Choose an item: (1) Intellinet 150N or (2) Wintec PCI" << endl;
      cin >> item;
      c.getItemPrice(category,item);
     
      
        cout << "Enter in router category number: ";
      cin >> category;
      cout << "Choose an item: (1) Asus N66U or (2) D Link 655" << endl;
      cin >> item;
      c.getItemPrice(category,item);
      
      
         cout << "Enter in laptop size category number: ";
      cin >> category;
      cout << "Choose an item: (1) 14' or (2) 16'" << endl;
      cin >> item;
      c.getItemPrice(category,item);

  13. #13
    Registered User rogster001's Avatar
    Join Date
    Aug 2006
    Location
    Liverpool UK
    Posts
    1,472
    i am not going through all that code but this -> operator is used to access members where the object is a pointer- you should use the dot operator otherwise
    Thought for the day:
    "Are you sure your sanity chip is fully screwed in sir?" (Kryten)
    FLTK: "The most fun you can have with your clothes on."

    Stroustrup:
    "If I had thought of it and had some marketing sense every computer and just about any gadget would have had a little 'C++ Inside' sticker on it'"

  14. #14
    Registered User
    Join Date
    Feb 2013
    Posts
    24
    ^ Thank you so much. I've finally gotten this to work the way i want it to!!!

  15. #15
    Registered User rogster001's Avatar
    Join Date
    Aug 2006
    Location
    Liverpool UK
    Posts
    1,472
    Glad you got it going, consider a few other points:

    I think it is time you broke this program into seperate source files and header(s) - And it will be a good execrcise to extend the project besides.

    Your main function code as it stands - you see all the repetition? You could investigate employing a switch and perhaps additional function calls.

    Finally - For me code formatting is a bit too inconsistent and the use of braces with code lines is not a style i would favour
    Thought for the day:
    "Are you sure your sanity chip is fully screwed in sir?" (Kryten)
    FLTK: "The most fun you can have with your clothes on."

    Stroustrup:
    "If I had thought of it and had some marketing sense every computer and just about any gadget would have had a little 'C++ Inside' sticker on it'"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help to tweak this beginner program :|
    By Sephyx in forum C++ Programming
    Replies: 9
    Last Post: 10-04-2011, 10:49 PM
  2. can someone look at my code and provide suggestions ?
    By atlas07 in forum C++ Programming
    Replies: 16
    Last Post: 05-13-2010, 05:23 AM
  3. Suggestions for this code? and 1 question....
    By Huskar in forum C Programming
    Replies: 6
    Last Post: 03-31-2009, 09:24 AM
  4. tweak CMOS settings
    By yin_howe_ho in forum Windows Programming
    Replies: 3
    Last Post: 09-30-2003, 03:11 PM
  5. homework code/looking for suggestions
    By m.albert in forum C++ Programming
    Replies: 3
    Last Post: 03-03-2003, 03:50 PM