I have created a basic object oriented database of a list of shoes containing the shoe name, number and shoe size. I am trying to sort the list of shoes alphabetially by name and by shoe size. I have the following code:

Shoes.h:

#include <iostream>
#include <fstream>
#include <string>

#ifndef Shoes_Shoes_h
#define Shoes_Shoes_h

using namespace std;

class Shoe
{
public:

    //Constructors
    Shoe();
    Shoe(string Name_, double ShoeSize_, int Number_);

    //Destructor
    ~Shoe();

    //Getters
    string getName() const;
    double getShoeSize() const;
    int getNumber() const;

    //Setters
    void setName(const string Name_);
    void setShoeSize(const double ShoeSize_);
    void setNumber(const int Number_);

    //Variables
    string Name;
    double ShoeSize;
    int Number;

};

class Shoe_List
{
public:

    //Constructor
    Shoe_List();

    //Functions
    void load();
    void add();
    void save();
    void sort();
    int sortname(const void*, const void*);
    int sortshoesize(const void*, const void*);
    void view();

private:

    int length;
    Shoe * shoes;
};

#endif

Shoes.cpp:

#include <iostream>
#include <fstream>
#include <string>

#include "Shoes.h"

using namespace std;

//Shoe Constructors
Shoe::Shoe()
{
    Name = "";
    ShoeSize = 0;
    Number = 0;
}

Shoe::Shoe(string Name_, double ShoeSize_, int Number_)
{
    Name = Name_;
    ShoeSize = ShoeSize_;
    Number = Number_;
}

//Shoe Destructor
Shoe::~Shoe()
{

}

//Shoe Getters
string Shoe::getName() const
{
    return Name;
}

double Shoe::getShoeSize() const
{
    return ShoeSize;
}

int Shoe::getNumber() const
{
    return Number;
}

//Shoe Setters
void Shoe::setName(const string Name_)
{
    Name = Name_;
}

void Shoe::setShoeSize(const double ShoeSize_)
{
    ShoeSize = ShoeSize_;
}

void Shoe::setNumber(const int Number_)
{
    Number = Number_;
}

//Shoe_List Constructor
Shoe_List::Shoe_List()
{
    shoes = NULL;
    int length = 0;
}

void Shoe_List::load()
{

    fstream fin; 

    fin.open("shoe_list.txt");

    if(fin.good()) 
    {
        fin >> length;

        shoes = new Shoe[10];

        for (int i=0; i<length; i++)
        {
            fin >> shoes[i].Name;
            fin >> shoes[i].Number;
            fin >> shoes[i].ShoeSize;
        }
    }

    fin.close();

}

void Shoe_List::add()
{
    if (length == 10) 
    {
        cout << "Database is full" << endl;
    }

    else 
    {
        cout << "Add: " << endl;
        cin >> shoes[length].Name;
        cin >> shoes[length].Number;
        cin >> shoes[length].ShoeSize;
        cout << endl;
        length++;
    }
}

void Shoe_List::save()
{
    fstream fout; 

    fout.open("shoe_list.txt", ios::out);

    if(fout.good()) 
    {
        fout << length << endl << endl;

        for (int i=0;i<length;i++) 
        {
            fout << shoes[i].Name << endl;
            fout << shoes[i].Number << endl;
            fout << shoes[i].ShoeSize << endl << endl;
        }
    }

    fout.close();
}

void Shoe_List::view()
{

    for (int i=0; i<length; i++)
    {
        cout << shoes[i].Name << endl;
        cout << shoes[i].Number << endl;
        cout << shoes[i].ShoeSize << endl;
        cout << endl;
    }
}

void Shoe_List::sort()
{
    char input;

    cout << endl;
    cout << "Sorting options" << endl;
    cout << "1. Sort by Name" << endl;
    cout << "2. Sort by ShoeSize" << endl;
    cout << "3. No sorting" << endl << endl;

    cout << "Enter option: ";
    cin >> input;
    cout << endl;

    switch(input) 
    {
        case '1':
            qsort(shoes, length, sizeof(shoes), sortname);
            break;

        case '2':
            qsort(shoes, length, sizeof(shoes), sortshoesize);
            break;

        case '3':
            return;

        default:
            cout << "Invalid entry" << endl << endl;
            break;
    }
}

int Shoe_List::sortname(const void *First, const void* Second)
{

    return strcmp(((shoes *) First)->Name, ((shoes *) Second)->Name);

}

int Shoe_List::sortshoesize (const void *First, const void* Second)
{

    return (int) ((((shoes *) First)->ShoeSize > ((shoes *) Second)->ShoeSize)? 1 : -1);

}

main.cpp:

#include <iostream>
#include <fstream>
#include <string>

#include "Shoes.h"

using namespace std;

int main()
{
    Shoe_List shoes;

    char input;

    while (1) 
    {

        cout << endl;
        cout << "Menu" << endl;
        cout << "1. Load" << endl;
        cout << "2. Add" << endl;
        cout << "3. Save" << endl;
        cout << "4. Sort" << endl;
        cout << "5. View" << endl;
        cout << "6. Exit" << endl;

        cout << "Enter option: ";
        cin >> input;
        cout << endl;
        // act on the user's input
        switch(input) 
        {
            case '1':
                shoes.load();
                break;

            case '2':
                shoes.add();
                break;

            case '3':
                shoes.save();
                break;

            case '4':
                shoes.sort();
                break;

            case '5':
                shoes.view();
                break;

            case '6':
                return 0;
                break;

            default:
                cout << "Incorrect entry" << endl << endl;
                break;
        }
    }
}

I keep getting the following errors in Shoes.cpp:

(162): error C3867: 'Shoe_List::sortname': function call missing argument list; use '&Shoe_List::sortname' to create a pointer to member

(166): error C3867: 'Shoe_List::sortshoesize': function call missing argument list; use '&Shoe_List::sortshoesize' to create a pointer to member

(181): error C2059: syntax error : ')'

(188): error C2059: syntax error : ')'

Could anyone see why this is the case or suggest a better way round this?

Sorry, but if you want to use qsort() then the comparision functions must either be static members or non-members. A pointer to a member function means something quite different than a pointer to a static member function or non-member function, and because of this they're incompatible.

The good news is they don't need to be non-member functions, they don't depend on any object except the ones provided as arguments:

int sortname(const void *First, const void *Second)
{
    const Shoe *pFirst = (const Shoe*)First;
    const Shoe *pSecond = (const Shoe*)Second;

    return strcmp(pFirst->Name, pSecond->Name);
}
int sortshoesize (const void *First, const void *Second)
{
    const Shoe *pFirst = (const Shoe*)First;
    const Shoe *pSecond = (const Shoe*)Second;

    // Don't forget about equality
    if (pFirst->ShoeSize == pSecond->ShoeSize) {
        return 0;
    }

    return pFirst->ShoeSize > pSecond->ShoeSize ? 1 : -1;
}

Ok but this still gives me the following errors:

error C3867: 'Shoe_List::sortname': function call missing argument list; use '&Shoe_List::sortname' to create a pointer to member

error C3867: 'Shoe_List::sortshoesize': function call missing argument list; use '&Shoe_List::sortshoesize' to create a pointer to member

error C2664: 'strcmp' : cannot convert parameter 1 from 'const std::string' to 'const char *'

Maybe I am not understanding you correctly?

Maybe I am not understanding you correctly?

Clearly not, because the errors state that sortname() and sortshoesize() are both still member functions of the Shoe_List class. Take them out of the class.

What would my options be then? create my own compare function?

What would my options be then?

Well, you could take the other route I mentioned of making the comparison functions static in your class. Then they're still members, but meet the requirements of qsort().

What would you suggest? Would it be good practice to do this?

What would you suggest?

Did my previous posts get deleted or something? I've already made two suggestions for making the code work within the restrictions of your current design.

Would it be good practice to do this?

Not really, but using qsort isn't the best practice either. Nor is having I/O in your sort() function. Honestly, if you're having trouble making things work, you should be trying whatever comes to mind to build experience instead of worrying about best practice.

There's no need to be so blunt or arrogant; well that is how you are coming across to me anyway. Im here to learn after all. Maybe that should have been 'What ELSE would you suggest?' I'm not worrying about best practice I'm only asking since there is no harm in learning this along the way.

There's no need to be so blunt or arrogant; well that is how you are coming across to me anyway.

I get progressively more annoyed when it seems like my advice is being ignored for no good reason.

Maybe that should have been 'What ELSE would you suggest?'

Make Shoe comparable in that it has an overloaded operator<, then use std::sort() from <algorithm>. Better yet, dump Shoe_List altogether and use std::list<Shoe>.

One of the better options would be to do this:

class Shoe
{
public:
    //Constructors
    Shoe();
    Shoe(string Name_, double ShoeSize_, int Number_);

    //Destructor
    ~Shoe();

    //Getters
    string getName() const;
    double getShoeSize() const;
    int getNumber() const;

    //Setters
    void setName(const string& Name_);
    void setShoeSize(const double ShoeSize_);
    void setNumber(const int Number_);

    //Variables
    string Name;
    double ShoeSize;
    int Number;

    static bool compareByName(const Shoe& lhs, const Shoe& rhs) {
      return lhs.Name < rhs.Name;
    };

    static bool compareBySize(const Shoe& lhs, const Shoe& rhs) {
      return lhs.ShoeSize < rhs.ShoeSize;
    };
};

And then, your Shoe_List::sort() should use std::sort() instead of qsort (which is an old C sorting function, not recommended in C++):

void Shoe_List::sort()
{
    char input;
    cout << endl;
    cout << "Sorting options" << endl;
    cout << "1. Sort by Name" << endl;
    cout << "2. Sort by ShoeSize" << endl;
    cout << "3. No sorting" << endl << endl;
    cout << "Enter option: ";
    cin >> input;
    cout << endl;
    switch(input) 
    {
        case '1':
            std::sort(shoes, shoes + length, Shoe::compareByName);
            break;
        case '2':
            std::sort(shoes, shoes + length, Shoe::compareBySize);
            break;
        case '3':
            return;
        default:
            cout << "Invalid entry" << endl << endl;
            break;
    }
}

And, as deceptikon pointed out, the even better practice is not to use a C-style array (Shoe* shoes;) but rather a C++ standard container such as std::vector<Shoe> shoes; in this case.

Ok for some reason when I use sort from the code you have provided it does't sort properly. It does lists them in a different order with no ascending pattern :-S

This article has been dead for over six months. Start a new discussion instead.