This topic is 2740 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi guys, my first post in this forum! I've been seeing this website/forum for a short time, and I decided to register myself and solve my questions (if they are solvable)

Ok, straight to the point.

I have this class :

#ifndef CALENDARIO_H #define CALENDARIO_H class Calendario { public : Calendario(); Calendario(const int& month, const int& day) : currentMonth(month), currentDay(day) {}; ~Calendario(); // Operator I want to overload Calendario& operator+(const int&); // Getters int getCurrentMonth() const { return currentMonth; } int getCurrentDay() const { return currentDay; } // Setters void setCurrentMonth(int& month) { currentMonth = month; } void setCurrentDay(int& day) { currentDay = day; } private : int currentMonth; int currentDay; }; #endif /* CALENDARIO_H */ 

And the implementation of the function member that is causing me troubles :

Calendario& Calendario::operator+(const int& y) { if ((y + currentDay) < currentMonth) // With the tests I was doing, at least this if should be executed currentDay += y; else if ((y + currentDay) > currentMonth) { for (int i = 0; i < y; ++i) { currentDay += 1; if (currentDay == currentMonth) { currentDay = 0; currentMonth += 1; } } } return *this; }

and the main :

int main(int argc, char** argv) { Calendario * cal = new Calendario(10, 5); //cout << cal->getCurrentDay() << endl; // prints 5 //cout << cal->getCurrentMonth() << endl; // prints 10 Calendario * calsum = cal + 2; cout << calsum->getCurrentDay(); // prints 0 delete cal; return 0; } 

For what I understand about overloading operators, that should work, but it doesn't. I'm adding 2 to currentDay of the object cal, and then I'm trying to show it by calling calsum->getCurrentDay(), but it prints 0, and I can't understand why. By overloading the operator+ I'm trying to modify the value of currentDay and currentMonth.

These lines :

 Calendario * cal = new Calendario(10, 5); // currentMonth = 10, currentDay = 5 Calendario * calsum = cal + 2; // currentMonth = 10, currentDay = 5 (+2 = 7) 

So, I create a Calendario object with the value 10 for currentMonth and 5 for currentDay, then, I'm adding 2 to currentDay (for the code of the overloading operator), later, in the overloading operator :

 if ((y + currentDay) < currentMonth) // ( (2 + 5) < 10) = true currentDay += y; // 5+2 = 7 , now currentDay should be 7 

Since y + currentDay (7) are lower than currentMonth (10), the new value of currentDay should be 7. After that, I try to print the new value :

 Calendario * calsum = cal + 2; cout << calsum->getCurrentDay();

but it prints 0, and like I've already said at the beggining... I don't know why...

I think I repeat a lot of things trying to explain my error, but now my head is about to explode .

I hope you can help me, and tell me, if you don't mind, what I'm doing wrong.

I'm reading The C Programming Language by Bjarne Stroustrup, and damn... that's a heavy book for a beginner like me . I'm in the chaper Operator Overloading, and till now, i see that my code should work.

Well... I don't know what else to say... if you need more info, just let me know.

PS : Sorry for my english.

##### Share on other sites
First of all you don't need const int&, just a simple (int x) would be suffice, const T& it for classes not primitive types. Also imho, you shouldn't really be using operating+ overloading here. Just write a function to add days to it, take days etc. Because simply just doing cal + 2 doesn't make much sense. Why do you really need for the class to be a pointer?

##### Share on other sites
That's not a valid way to overload the + operator, that's actually almost exactly how to overload +=

See the very useful list of operator overloads on Wikipedia, here
Also as noted, do not pass built-in types by const-reference, pass them by value instead.

Edit: Wow, this forum uses a different method of creating links and I got caught out!

##### Share on other sites
Calendario * cal = new Calendario(10, 5); ... Calendario * calsum = cal + 2;

##### Share on other sites
As said before, you should change the operator to operator+=. This way you can easily implement operator+:
 class Calendario { ... static Calendario operator+(Calendario lhs, Celandario rhs) { Calendario tmp = lhs; tmp += rhs; return tmp; } }; 

Your second problem is that for some reason, you are creating your Calendario instance with new, which isn't required at all.
The problem is that the expression "cal + 2" does NOT use your own operator+, since cal is NOT of the type Calendario, but
Calendario* instead, which is a pointer to a Calendario object. This means that c++ pointer arithmetic kicks in. The value of the pointer
is incremented by 2 times the size of a Calendario object. In your case this will most likely be 2*8 bytes. This produces undefined behaviour
in your code (which is really bad), since you only allocated one instance and the memory after that can be filled with anything, most probably random memory.

It would be easier to create your Calendario objects on the stack:

 int main(int argc, char* argv[]) { Calendario cal(10,5); Calendario calsum = cal + 2; cout << calsum.getCurrentDay(); return 0; } 

##### Share on other sites
Calendario * cal = new Calendario(10, 5); // currentMonth = 10, currentDay = 5
Calendario * calsum = cal + 2; // currentMonth = 10, currentDay = 5 (+2 = 7)[/quote]

cal is actually of type "Calendario pointer" so when you add 2 to it, you're doing pointer arithmetic. calsum is actually pointing to uninitialised memory, so any attempt to use it will result in a crash, debug exception, strange behaviour, etc.

To add 2 to the object itself and not it's pointer, you need to use the dereference operator, *. But in order to use these operators properly, in the way I think you're trying to use them, you should be creating Calendario objects by value and not using the new operator.

 Calendario cal(10,5); // Now cal is an actual object and not just a pointer to one Calendario calsum = cal + 2; 

For this to work, the + operator should be returning an entirely new Calendario object, and not a reference to itself.

##### Share on other sites
Wow... that actually worked... I'm such a.... well, I'm just learning, but I should do it well xD.

So, what I've learned today :

- I shouldn't pass built-in types by const.-reference
- The way I was trying to use the operator + with the pointer to Calendario class, is actually pointer arithmetic, so it never calls the operator+ overloaded in the class.
- And finally, the operator+ should return a Calendario object, and not a reference to itself.

I've learned a lot in 5 asnwers, much more than I do reading the book . It's in english, so it cost me a lot to understand all I read.

Thanks a lot guys, thanks for wasting your time answering this noobie question.

By the way, I'm overloading the operator + instead of using a function, just because I want to learn to do this kind of things. Obviously, eventually, I'll do it with a function, but I need to learn to do all these kinds of things. This is just for learning, It's not a homework or anything like that.

See you .

##### Share on other sites

First of all you don't need const int&, just a simple (int x) would be suffice, const T& it for classes not primitive types.

I would disagree The compiler should be able to detect that the primitive is not changed in the function but you can help it make this decision by applying const. [s]Once you apply const you can not pass a literal so you then need a int const &.[/s]

##### Share on other sites
What? That makes no sense. Why on earth would having a reference to an int help in this situation?

##### Share on other sites

What? That makes no sense. Why on earth would having a reference to an int help in this situation?

I'm sorry your comment makes no sense unless you were looking at the post before I made the edit , which was almost immediately.

1. 1
Rutin
19
2. 2
3. 3
JoeJ
16
4. 4
5. 5

• 26
• 20
• 13
• 13
• 17
• ### Forum Statistics

• Total Topics
631700
• Total Posts
3001782
×