[SOLVED] C++ Operator Overloading Within an Already Overloaded Operator

mirlin510 · 04-16-2011, 08:40 PM

I'm having a bit of an issue using overloaded operators in an already overloaded operator. In my following code, I have overloaded the && operator to compare two Course objects. The operator in turn goes to a function which calls other overloaded operators to compare private object variables of that object to compare them mainly:

Code:

bool operator&&(const DaysOfWeek& a, const DaysOfWeek& b);
bool operator&&(const TimeInterval& a, const TimeInterval& b);

Now, for my question. I have been using many overloaded operators in this project, but this is the first time I have had to call overloaded operators inside other overloaded operators. Unfortunately, my overloaded operators in the code above is not being called from my isOverlap function. So my question is: why is this and how can I correct it?

Any help would be GREATLY appreciated, as I am banging my head against the wall trying to get this to work. i have included the relevent code from Course.h and the functions and overloaded operator in Course.cpp. I have bolded the appropriate lines of code that I am having irregular output for (not using my overloaded operator).

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(this->instructor==b.getInstructor() &&
        &this->days&&(&tempDays) &&
        &this->time&&(&tempTime))
    {
        return true;
    }
    else
        return false;
}

bool operator&&(const Course& a, const Course& b)
{
    if (a.isOverlap(b))
        return true;
    else
        return false;
}

Code:

#ifndef COURSE_H
#define COURSE_H

#include <string>
#include "TimeInterval.h"
#include "DaysOfWeek.h"

using namespace std;

class Course
{
    public:
        Course();
        Course(const string courseCode, const string section,
            const DaysOfWeek& days, const TimeInterval& time,
            const string instructor);
        void setCourse(string courseCode, string section,
            DaysOfWeek& days, TimeInterval& time, string instructor);
        string getCourse() const;
        string getSection() const;
        DaysOfWeek getDays() const;
        TimeInterval getTime() const;
        string getInstructor() const;
        bool isOverlap(const Course& b) const;
        bool isMatch(const Course& b) const;

    private:
        string courseCode;
        string section;
        DaysOfWeek days;
        TimeInterval time;
        string instructor;
};

bool operator&&(const Course& a, const Course& b);
bool operator==(const Course& a, const Course& b);

#endif //COURSE_H

SigTerm · 04-16-2011, 09:34 PM

Advice: read C++ book and get more practice.
The problem is with this part:

Quote:

Originally Posted by mirlin510

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(this->instructor==b.getInstructor() &&
        &this->days&&(&tempDays) &&
        &this->time&&(&tempTime))
    {
        return true;
    }
    else
        return false;
}

Problems:

You don't have to use "this->". C++ isn't python.
"&" before variable (&i) means "take address of variable". The result is a pointer, and any non-zero pointer converts to bool (non zero pointer converts to "true" value). so (&i && &j) will always return true (unless you use some trickery and undefined behavior).
Your code might cause infinite recursion and stack overflow, so program will crash.
You use references, not pointers, you don't need to take address of variable to pass it as a reference.
(subjective) You use && operator to find if two ranges "intersect", as I understand. This is not exactly same thing as "logical AND", in my opinion, but that's up to you. I'd recommend to convert "tell if two ranges intersect" operator into (non-operator) function. IMO, there's no need for operator.

For a start, try replacing this:

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(this->instructor==b.getInstructor() &&
        &this->days&&(&tempDays) &&
        &this->time&&(&tempTime))
    {
        return true;
    }
    else
        return false;
}

With this:

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(instructor==b.getInstructor() &&
        days && tempDays &&
        time && tempTime)
    {
        return true;
    }
    else
        return false;
}

And see what happens.

mirlin510 · 04-16-2011, 09:49 PM

Quote:

Originally Posted by SigTerm

Advice: read C++ book and get more practice.
The problem is with this part:

Problems:

You don't have to use "this->". C++ isn't python.
"&" before variable (&i) means "take address of variable". The result is a pointer, and any non-zero pointer converts to bool (non zero pointer converts to "true" value). so (&i && &j) will always return true (unless you use some trickery and undefined behavior).
Your code might cause infinite recursion and stack overflow, so program will crash.
You use references, not pointers, you don't need to take address of variable to pass it as a reference.
(subjective) You use && operator to find if two ranges "intersect", as I understand. This is not exactly same thing as "logical AND", in my opinion, but that's up to you. I'd recommend to convert "tellIfTwoCoursesIntersects" operator in standard function. There's no need for operator, but that's just my opinion.

For a start, try replacing this:

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(this->instructor==b.getInstructor() &&
        &this->days&&(&tempDays) &&
        &this->time&&(&tempTime))
    {
        return true;
    }
    else
        return false;
}

With this:

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(this->instructor==b.getInstructor() &&
        days && tempDays &&
        time && tempTime)
    {
        return true;
    }
    else
        return false;
}

And see what happens.

First, thank you very much for responding!

I tried doing your replacement and it gave me the compiler error saying that my arguments do not match what I defined for my overloaded operator. If you have any other ideas, please let me know. They are greatly appreciated.

Quote:

You don't have to use "this->". C++ isn't python.

You're right! I didn't know that until now. I thought you had to in a member function.

You were also correct about the "&&" addressing twice as well. I will try to avoid this in the future.

Unfortunately the assignment forces me to use the && operator to compare two Course objects.

dwhitney67 · 04-16-2011, 10:08 PM

Quote:

Originally Posted by mirlin510

Unfortunately the assignment forces me to use the && operator to compare two Course objects.

To compare two object of the same type, you need to define the operator==() method; something like:

Code:

class Course
{
public:
   ...

   bool operator==(const Course& other) const;

private:
   ...
};

...


bool
Course::operator==(const Course& other) const
{
   if (this == &other) return true;

   // compare member data of Course object (this) versus the member data of 'other' object...

   // "this" and "other" member data are not the same
   return false;
}

mirlin510 · 04-16-2011, 10:12 PM

Quote:

Originally Posted by dwhitney67

To compare two object of the same type, you need to define the operator==() method; something like:

Code:

class Course
{
public:
   ...

   bool operator==(const Course& other) const;

private:
   ...
};

...


bool
Course::operator==(const Course& other) const
{
   if (this == &other) return true;

   // compare member data of Course object (this) versus the member data of 'other' object...

   // "this" and "other" member data are not the same
   return false;
}

I have 8 other overloading operators and they all work across many classes. the rest use <, >, <=, >=, ==, !=, ||.

As long as the operator takes two objects, it works fine. In fact, like I said, I used && to get to where I am stuck currently from comparing two Courses.
I used the following code to get to the isOverlap function in the overloaded && operator:

Quote:

if (scheduler[i]&&scheduler[j] && days != "O")

This compares two Course objects in the scheduler vector.

mirlin510 · 04-16-2011, 10:59 PM

Had someone else find an answer to this, you must use parens around every single (variable&&variable) statement, as it will break with the other && operator in an if-statement.

dwhitney67 · 04-17-2011, 07:28 AM

Quote:

Originally Posted by mirlin510

This compares two Course objects in the scheduler vector.

I would not recommend that you use "&&" to denote a comparison for equality. Whereas it may seem fine to you at the moment, on a future date it may confuse you, and it will definitely confuse a new reader of your code.

Code:

if (scheduler[i]&&scheduler[j] && days != "O")

From my perspective, the code above implies that you are testing both scheduler[i] and scheduler[j] to see if they are "true", and also to test whether days is not equal to "O". If some other operation is taking place (such as you have indicated), it is not obvious. I'm not sure why you could not use the "==" operator to check for equality.

Don't get carried away with defining overloaded operators merely for the sake of it; it could lead to poor coding.

johnsfine · 04-17-2011, 08:07 AM

Quote:

Originally Posted by mirlin510

Unfortunately the assignment forces me to use the && operator to compare two Course objects.

I think you misunderstood the instructions.

I don't think you were told to write an operator&&() that compares two objects.

I think you were told to write an operator==() that compares two objects, and inside that operator==() you should use the ordinary (int or bool) && to combine the results of comparing members.

Before you turn in a mess due to that misunderstanding, do you have an opportunity to ask the instructor for clarification?

Quote:

Originally Posted by mirlin510

Had someone else find an answer to this, you must use parens around every single (variable&&variable) statement, as it will break with the other && operator in an if-statement.

That sounds like someone gave you a way to fix that problem without giving you any understanding of why.

Assume for the moment, you really do want to define a function that acts like operator==() but is named operator&&(), then you want to use it in an expression like:

x1 && x2 && y1 && y2

where the && should be the weird operator&&() you defined, while the && should be the ordinary && combining the bool results.

The fact that you defined operator&&() doesn't change the operator precedence of &&, so writing
x1 && x2 && y1 && y2
has the same meaning as
( (x1 && x2) && y1 ) && y2
which is not what you want. So you need parens around (y1 && y2) to correct the order of evaluation. Parens around (x1 && x2) aren't needed, but certainly help to make the weird intent of the code a little clearer.

mirlin510 · 04-17-2011, 12:02 PM

Quote:

Originally Posted by johnsfine

I think you misunderstood the instructions.

I don't think you were told to write an operator&&() that compares two objects.

I think you were told to write an operator==() that compares two objects, and inside that operator==() you should use the ordinary (int or bool) && to combine the results of comparing members.

Before you turn in a mess due to that misunderstanding, do you have an opportunity to ask the instructor for clarification?

That sounds like someone gave you a way to fix that problem without giving you any understanding of why.

Assume for the moment, you really do want to define a function that acts like operator==() but is named operator&&(), then you want to use it in an expression like:

x1 && x2 && y1 && y2

where the && should be the weird operator&&() you defined, while the && should be the ordinary && combining the bool results.

The fact that you defined operator&&() doesn't change the operator precedence of &&, so writing
x1 && x2 && y1 && y2
has the same meaning as
( (x1 && x2) && y1 ) && y2
which is not what you want. So you need parens around (y1 && y2) to correct the order of evaluation. Parens around (x1 && x2) aren't needed, but certainly help to make the weird intent of the code a little clearer.

Again this was solved, but like I said; I had to use == to find an exact match in course/instructor/time, while && is used to find overlap in times/courses/instructor. For example, a course that meets on MW at 8:30 to 9:30 with the same instructor would overlap with WF at 9:15 to 10:15 with the same instructor. They are not equal, but they do overlap. The instructor is notorious for being an idiot, so it's quite normal for him to write a garbage assignment. The assignment is to practice overloading operator, but he wrote over 20 overloaded operators...

Quote:

So you need parens around (y1 && y2) to correct the order of evaluation. Parens around (x1 && x2) aren't needed, but certainly help to make the weird intent of the code a little clearer.

This is what I meant, my final code looks like this:

Code:

bool Course::isOverlap(const Course& b) const
{
    DaysOfWeek tempDays = b.getDays();
    TimeInterval tempTime = b.getTime();
    if(instructor==b.getInstructor() && (days&&tempDays) &&
        (time&&tempTime))
    {
        return true;
    }
    else
        return false;
}