Thread: Help with getting a string to display.

I've got a program that's supposed to take a description, serial number and price for an item. Then sort by highest price, and display the new order.

Everything works(thanks to several dozen searches through the board/source codes here) except for displaying the description string at the end. I'm thinking(and I may be totally off base here) that it's something to do with buffers, but I just can't figure it out.

Any help would be greatly appreciated. THANKS!

Code:

#include <iostream>
#include <string>
#include <iomanip>

#define SWAP(a,b)   { double t; t=a; a=b; b=t; }  // Macros for Bubble Sort swapping
#define STRINGSWAP(c,d)   {string t; t=c; c=d; d=t;} 
#define INDEX 8

using namespace std;

const int MAX_NUM_PURCHASES = 50;
void get_all_purchases(double price[], double serialNum[], string descript[], int numPurchases);
void bubble_srt(double price[], double serialNum[], string descript[], int numPurchases);
void print_top_purchases(double price[], double serialNum[], string descript[], int numPurchases);


int main()
{

    double price[MAX_NUM_PURCHASES];
    double serialNum[MAX_NUM_PURCHASES];  
    string descript[MAX_NUM_PURCHASES];
    int numPurchases;
    
    do
    {
       cout << "How many purchases do you have? ";
       cin >> numPurchases;
       
       if (numPurchases <= 0 || numPurchases > MAX_NUM_PURCHASES)
            cout << "Must be between 1 and "<< MAX_NUM_PURCHASES << "\n";
    }while (numPurchases > MAX_NUM_PURCHASES || numPurchases <= 0);
    
    get_all_purchases(price, serialNum, descript, numPurchases);
    
    
    

    system("pause");
    return (0);
}


void get_all_purchases(double price[], double serialNum[], string descript[], int numPurchases)
{
    for (int i=0; i < numPurchases; i++)
    {
        cout << "\n\nInput the description: ";
        getline(cin, descript[i]);
        std::cin.clear();       
        std::cin.sync();      
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        cout << "Input the serial number: ";
        cin >> serialNum[i];
        do
        {
            cout << "Input the price: ";
            cin >> price[i];
            if(price <= 0)
                     cout << "Must be greater than zero\n";
        }while(price[i] <=0);
    }//! End of for loop over items-purchased input.
    bubble_srt(price, serialNum, descript, numPurchases);
    return; 
} 


/*
Bubble Sort source code found at cprogramming.com
http://www.cprogramming.com/source/bubblesort.c
*/
void bubble_srt( double price[], double serialNum[], string descript[], int numPurchases )  
{   
    int i, j;
       
    for(i = 0; i < numPurchases; i++)         // Make a pass through the array for each element
    {              
        for(j = 1; j < (numPurchases-i); j++) // Go through the array beginning to end
        {              
           if(price[j-1] < price[j])          // If the the first number is greater, swap it 
           {       
              SWAP(price[j-1],price[j]);
              SWAP(serialNum[j-1],serialNum[j]);
              STRINGSWAP(descript[j-1],descript[j]);
           }   
        }
    }//! End of for loop passing through each element
    print_top_purchases(price, serialNum, descript, numPurchases);
}

void print_top_purchases(double price[], double serialNum[], string descript[], int numPurchases)
{
     int numPrint;
     
     if(numPurchases < 5)
          numPrint = numPurchases;
     else
          numPrint = 5;
          
     for(int i = 0; i < numPrint; i++)
     {
             cout << (i+1) << ". " << setw(6) << price[i] << "   " << setw(6) 
             << serialNum[i] << "   " << setw(15) << descript[i] << endl;  /*this is where it    doesn't work, searched for hours, can't figure it out.*/
     }
}

Originally Posted by dwks

You never assign anything to descript in main(), so it won't print any descriptions.

It's assigned a value in get_all_values, which calls bubble_sort, which calls print_top_purchases. Since it's passed through each function as a parameter, shouldn't it survive and be able to be printed? Both price and serialNum make it, and they're assigned in the same place as descript.

Sorry, I'm not trying to be contrary, just want to understand .

Originally Posted by swoopy

Consequently, the program is skipping right over the getline. There are several different ways to fix this, but the idea is to use cin.ignore to eat the newline still in the input buffer. For example, you could move the cin.ignore above the getline.

Close! I tried that, and it didn't work. So I just tried something on a whim.

I just deleted the

Code:

        std::cin.clear();       
        std::cin.sync();

part. Now it works perfectly. I guess the cin.clear() was actually clearing out everything I put in

This is the new function.

Code:

void get_all_purchases(double price[], double serialNum[], string descript[], int numPurchases)
{
    for (int i=0; i < numPurchases; i++)
    {
        cout << "\n\nInput the description: ";      
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        getline(cin, descript[i]);
        cout << "Input the serial number: ";
        cin >> serialNum[i];
        do
        {
            cout << "Input the price: ";
            cin >> price[i];
            if(price <= 0)
                     cout << "Must be greater than zero\n";
        }while(price[i] <=0);
    }//! End of for loop over items-purchased input.
    bubble_srt(price, serialNum, descript, numPurchases);
    return; 
}

Thanks for the help!

Please excuse my rudeness, System_159. I'm going to point out some things I think could be improved in your program.

Originally Posted by System_159

Code:

#define SWAP(a,b)   { double t; t=a; a=b; b=t; }  // Macros for Bubble Sort swapping
#define STRINGSWAP(c,d)   {string t; t=c; c=d; d=t;}

I'm told that macros are bad in C++. You should avoid them. Templates and inlining can replace both of those macros with one function.

Code:

template <class Type>
void Swap(Type& a, Type& b)
{
  Type temp = a;

  a = b;
  b = temp;
}

This is good, but C++ already has a swap function just like it if you include <algorithm>.

Originally Posted by System_159

Code:

using namespace std;

This should never be used in the global scope. It was designed to help older code transition to newer compilers with namespaces, not for making new code more convenient. You should only do it inside of functions and only with the smallest possible scope. What I do is use std:: with the name if I only use it once and a "using std::name;" with the smallest scope if I use it more than once.

Originally Posted by System_159

Code:

    double price[MAX_NUM_PURCHASES];
    double serialNum[MAX_NUM_PURCHASES];  
    string descript[MAX_NUM_PURCHASES];

When you have more than one array and they're all related, you should be using a class. Then you can have an array of objects that are easier to use.

Code:

class Purchase
{
public:
    Purchase(double cost, double serial, string descr)
        : price(cost), serialNum(serial), description(descr) {}

    // stuff users can do with a purchase
private:
    double price;
    double serialNum;
    string description;
};

Originally Posted by System_159

Code:

    int numPurchases;
    
    do
    {
       cout << "How many purchases do you have? ";
       cin >> numPurchases;
       
       if (numPurchases <= 0 || numPurchases > MAX_NUM_PURCHASES)
            cout << "Must be between 1 and "<< MAX_NUM_PURCHASES << "\n";
    }while (numPurchases > MAX_NUM_PURCHASES || numPurchases <= 0);

C++ has a vector class that makes arrays easier to use. You should prefer C++ classes and functions over doing things manually because it's shorter, safer, and probably faster too.

Originally Posted by System_159

Code:

void get_all_purchases(double price[], double serialNum[], string descript[], int numPurchases)
{
    for (int i=0; i < numPurchases; i++)
    {
        cout << "\n\nInput the description: ";
        getline(cin, descript[i]);
        std::cin.clear();       
        std::cin.sync();      
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        cout << "Input the serial number: ";
        cin >> serialNum[i];
        do
        {
            cout << "Input the price: ";
            cin >> price[i];
            if(price <= 0)
                     cout << "Must be greater than zero\n";
        }while(price[i] <=0);
    }//! End of for loop over items-purchased input.
    bubble_srt(price, serialNum, descript, numPurchases);
    return; 
}

You don't need cin.clear or cin.sync, and a better place for bubble_srt is in main. If you're allowed to do it, come up with a way to set up purchases so that they're all on one line. Then you can read a line with getline and break it up with a stringstream. If you get rid of the cout messages then you can take an ostream as a parameter and be able to use this function for reading from a file too.

Originally Posted by System_159

Code:

/*
Bubble Sort source code found at cprogramming.com
http://www.cprogramming.com/source/bubblesort.c
*/
void bubble_srt( double price[], double serialNum[], string descript[], int numPurchases )  
{   
    int i, j;
       
    for(i = 0; i < numPurchases; i++)         // Make a pass through the array for each element
    {              
        for(j = 1; j < (numPurchases-i); j++) // Go through the array beginning to end
        {              
           if(price[j-1] < price[j])          // If the the first number is greater, swap it 
           {       
              SWAP(price[j-1],price[j]);
              SWAP(serialNum[j-1],serialNum[j]);
              STRINGSWAP(descript[j-1],descript[j]);
           }   
        }
    }//! End of for loop passing through each element
    print_top_purchases(price, serialNum, descript, numPurchases);
}

A better place for print_top_purchases is in main. C++ also already has a sort function that's a lot faster than bubble sort. If you use a class for purchases then you can overload the < operator and std::sort will work with it.

Originally Posted by System_159

Code:

void print_top_purchases(double price[], double serialNum[], string descript[], int numPurchases)
{
     int numPrint;
     
     if(numPurchases < 5)
          numPrint = numPurchases;
     else
          numPrint = 5;
          
     for(int i = 0; i < numPrint; i++)
     {
             cout << (i+1) << ". " << setw(6) << price[i] << "   " << setw(6) 
             << serialNum[i] << "   " << setw(15) << descript[i] << endl;
     }
}

numPrint should be a parameter, and if you use a vector then you don't have to pass the size. You can overload the << operator for your class and make the cout statement shorter.

Last, it's always better to use a namespace for all but main if you can. I'm pretty sure of that at least. I'm still waiting for a few people to confirm it, but it makes the most sense to me.

I ended up rewriting your program using my suggestions and it's completely different. I'm sorry.

Code:

#include <algorithm>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <sstream>
#include <string>
#include <vector>

// add interfaces to the namespace
namespace PurchaseProgram
{
    using std::vector;

    class Purchase;

    void getPurchases(std::istream& is, vector<Purchase>& purchases, char delim);
    void printFirstN(const vector<Purchase>& purchases, int n);
}

int main()
{
    std::vector<PurchaseProgram::Purchase> purchases;
    std::ifstream is("purchases.txt");

    if (is)
    {
        // print the 5 least expensive purchases
        getPurchases(is, purchases, '|');
        sort(purchases.begin(), purchases.end());
        printFirstN(purchases, 5);
    }

    return 0;
}

// add implementations to the namespace
namespace PurchaseProgram
{
    using std::string;
    using std::ostream;

    class Purchase
    {
    public:
        Purchase(double cost, double serial, string descr)
            : price(cost), serialNum(serial), description(descr) {}

        // overload less than to make Purchase sortable with std::sort
        bool operator<(const Purchase& purchase)
        {
            return price < purchase.price;
        }

        // let users write without breaking encapsulation
        friend ostream& operator<<(ostream& os, const Purchase& purchase)
        {
            using std::setw;

            return os << setw(6) << purchase.price << "   "
                << setw(6) << purchase.serialNum << "   "
                << setw(15) << purchase.description;
        }
    private:
        double price;
        double serialNum;
        string description;
    };

    void getPurchases(std::istream& is, vector<Purchase>& purchases, char delim)
    {
        string line;

        while (getline(is, line))
        {
            std::stringstream iss(line);
            string description;
            double serial;
            double price;

            // break up purchases with items separated by delim
            // ex. delim = '|', line = "Teddy Bear|99999|9.99"
            getline(iss, description, delim);
            iss >> serial;
            iss.get();
            iss >> price;

            // add a new purchase to the list
            purchases.push_back(Purchase(price, serial, description));
        }
    }

    void printFirstN(const vector<Purchase>& purchases, int n)
    {
        vector<Purchase>::const_iterator iter;
        int i = 0;

        for (iter = purchases.begin(); iter != purchases.end(); ++iter)
        {
            // don't print more than n purchases
            if (i++ >= n)
            {
                break;
            }

            std::cout << i << ". " << *iter << "\n";
        }
    }
}

Hikaru, not being rude at all. I'm a novice, and you're trying to point out my mistakes. I appreciate it very much.

I may be able to implement some of your suggestions, but as I'm doing this for a class I want to be sure I understand everything before I submit it. I've not done anything with templates or vectors before so I'll have to research before I put them in. And we're not supposed to be using classes yet (doing reveiw of procedural programming before moving on to OOD).

Thanks for the help, though!

Macros with #define are a throw back to the C language.
constants are used in place of them in C++.
As you get more confident and learn more, you will drop old-style program modes like
structs and macros and learn to use classes, constants and the STL, ( standard template library) a collection of vectors, itrerators and algoritthms to make life easier