Operator overloading troubles

Question

I am trying to compare a list of objects called Persons for a Quicksort. The object contains strings for first name, last name, SSN, and phone number, as well as some useful members.

class Person
{
protected:
string firstname, lastname, social, birthday;
 public:
Person(string, string, string, string); //store all values in strings for easy comparison and because no manipulation is necessary
~Person();  
string Get(int type); //allows retrieval of protected members of object
void Print();
bool operator > (const Person& pivot) const;
//bool operator < (const Person &pivot);
};

I am trying to use operator overloading to determined which of two Person objects is larger.

bool Person::operator > (const Person& pivot) const
{
if(this->firstname > pivot.firstname)
    return true;
if(this->firstname < pivot.firstname)
    return false;
if(this->lastname > pivot.lastname)
    return true;
if(this->lastname < pivot.lastname)
    return false;
if(this->birthday > pivot.birthday)
    return true;
if(this->birthday < pivot.birthday)
    return false;
if(this->social > pivot.social)
    return true;
if(this->social < pivot.social)
    return false;
}

However, this doesn't seem to be working at all because when I run

Person * B = new Person("1234", "Donny", "Smith", "123456780");
Person * pivot = new Person("9345", "John", "Phillip", "234598765");
if (B > pivot);
    cout << "hooray\n";
if(pivot > B)
    cout << "Not hooray\n";

the contents of both if statements are executed. I'm thoroughly confused, but it's probably some very stupid mistake.

score 0 · Answer 1 · answered Sep 18 '13 at 23:38

You declare B and pivot as a pointer. So, basically, your are comparing pointers. When you do:

if (B > pivot)

you are comparing memory address. To solve this, try doing:

if (*B > *pivot)

By the way, remove the semicolon from this statement:

if (B > pivot); // <- here

score 0 · Answer 2 · edited May 23 '17 at 12:28

0

First of all, as Avi noticed, there is a semicolon at the if statement.

Secondly, you are instancing the objects using pointers/dynamic-memory (In a Java style): Thats an error. In fact what you are comparing are the pointers, not the objects.

Dont use dynamic memory when is not necesary. Use local variables:

Person B("1234", "Donny", "Smith", "123456780");
Person pivot("9345", "John", "Phillip", "234598765");

if (B > pivot)
    std::cout << "hooray" << std::endl;
if(pivot > B)
    std::cout << "Not hooray" << std::endl;

Sidenote:

This kind of complex comparisons could be easily achieved through std::tie and std::tuple, as this thread shows: Implementing comparison operators via 'tuple' and 'tie', a good idea?

edited May 23 '17 at 12:28

Community

1
1

answered Sep 18 '13 at 23:40

Manu343726

13,969
4
40
75

I missed the error with the semicolon; that fixed the problem with both if statements running. However, I don't believe that the problem with the comparison lies with the fact that my Person objects are being created using new, because when I run the program now it still runs the wrong if statement (even if I create the Person objects locally). Additionally, tuple and tie are part of c++11 and boost; I believe the compiler I need to write for is using C++98. – musicman1007 Sep 18 '13 at 23:55
@musicman1007 allocate dynamically a local variable is an error, even if its not directly related with your problem. – Manu343726 Sep 18 '13 at 23:57
@musicman1007 check the implementation of `std::lexicographical_compare`, it could give you an idea about whats wrong with your code: http://en.cppreference.com/w/cpp/algorithm/lexicographical_compare – Manu343726 Sep 19 '13 at 00:02

Operator overloading troubles

2 Answers2

Sidenote: