0

I build a class mapping rational number, and i'd like to implement the multiplication by a scalar operation overflow, something like that :

Rational &operator*(Rational const& a, double lambda)
{
    Rational r(a._num * lambda, a._den);
}

Where _num and _den are the numerator and denominator of the rational number.

When I run cout << r * 45.2;, I get -858993460/4389064. It should be noted that the operator<< is working fine on others instances of Rational.

How can it's not be working? Thanks :)

Mateusz Grzejek
  • 11,698
  • 3
  • 32
  • 49
Rogue
  • 751
  • 1
  • 17
  • 36
  • 5
    How do you store `Rational`? What type are `_num` and `_den`? – Petr May 03 '15 at 12:43
  • and what's the value of r in cout? – phuclv May 03 '15 at 12:44
  • 1
    I'd hope that modern compilers will find both mistakes in your code and issue warnings for them, did you turn on warnings and consider and understand them? – Ulrich Eckhardt May 03 '15 at 12:45
  • _num and _den are both integers, and the value of r is a random test value, in this case it's 2 and 13 for example – Rogue May 03 '15 at 12:46
  • Try (a) compiling this with heightened warnings enabled (as high as you can get on your toolchain), and then (b) *fixing what is flagged.* At least one of the problems is obvious . – WhozCraig May 03 '15 at 12:47
  • I use Visual Studio, and i get no warnings except from some "Cannot find or open the PDB file." that i always get with any projects – Rogue May 03 '15 at 12:49
  • Hmmm .... seems this question has been marked as a duplicate. Problem is, the question it is marked as duplicating is a bit different. – Peter May 03 '15 at 12:53

3 Answers3

6

Rational & -- you're returning a reference to a temporary (undefined behavior) as well as failing to return it. You need:

Rational operator*(Rational const& a, double lambda) {
    return Rational(a._num * lambda, a._den);
}

I also recommend paying attention to your compiler warnings. The code you had originally should invoke multiple on decent compilers.

  • All right, seems like i didn't quite get the concept, i get it now, no references, just the actual variable. Thanks a lot! – Rogue May 03 '15 at 12:48
  • 1
    When returning references or pointers, you want to make sure that whatever you're returning won't be destroyed when exiting that scope. So when you implement an operator like `operator*=`, that's modifying the left hand side and returning it, so there you do typically return by reference. Cheers. –  May 03 '15 at 12:51
5

You don't return anything from that function. And you can't return references to local variables, so you should probably change the return type to Rational (remove the &)

Also, it's called "operator overloading"

weltensturm
  • 2,164
  • 16
  • 17
3

operator*() needs to return a value. Yours is not.

Any attempt by the caller to use the return value from a function that doesn't return one gives undefined behaviour.

Also, your function returns a reference. Which is probably not a good idea.

You probably want to do something like

Rational operator*(Rational const& a, double lambda)
{
    Rational r(a._num * lambda, a._den);
    return r;
}

Note that the return type is a Rational, not a reference. And that it returns the value of r.

This assumes that you have implemented the constructor being used to create r, and that it works as intended. You have provided no information that would confirm or deny that assumption.

Peter
  • 35,646
  • 4
  • 32
  • 74