2

I have the following in my header file:

template<typename T>
class rational {
private:
    T num;
    T denom;
    T gcd(T x, T y);

public:
    rational(T num, T denom);
    rational(T wholeNumber);

    template<typename U>
    friend inline rational<U> operator *(const rational<U> &lhs, const rational<U> &rhs);
}

template<typename T>
rational<T>::rational(T whole) {
    this->num = whole;
    this->denom = 1;
}

template<typename T>
rational<T> operator *(const rational<T> &lhs, const rational<T> &rhs) {
    return rational<T>(lhs.num * rhs.num, lhs.denom * rhs.denom);
}

And the following in my main:

rational<int> x(6), y(2);
rational<int> product = y * x;   // this works
rational<int> product2 = 2 * x;  // this doesn't

The first product works, but the second one gives me "error: no match for ‘operator*’ in ‘2 * x’". Why? Since there is a constructor available that takes only the 2 as an argument, shouldn't that be automatically called? If not, how else would I overload the operator to have both of these work?

Thanks.

David Thielke
  • 197
  • 1
  • 8
  • 2
    The reason why the conversion does not work, as well as a fix, is explained in [this answer](http://stackoverflow.com/a/3888237/160206). – Björn Pollex Feb 17 '12 at 07:59
  • possible duplicate of [Implicit conversion not happening](http://stackoverflow.com/questions/3888082/implicit-conversion-not-happening) – wilx Feb 17 '12 at 08:31

2 Answers2

0

I'm not sure why the compiler will not invoke the single-argument constructor implicitly on 2 in order to produce a rational, but my guess is that the inference mechanism is simply broken when templates are involved.

One workaround (if no one knows how to fix the implicit constructor issue) is to define an additional multiply operators like thus:

template<typename T>
rational<T> operator *(const T &lhs, const rational<T> &rhs) {
    return rational<T>(lhs * rhs.num, rhs.denom);
}
template<typename T>
rational<T> operator *(const rational<T> &lhs, const T &rhs) {
    return rational<T>(lhs.num * rhs, lhs.denom);
}

This will also perform better, if you are writing high-performance code using rationals in an inner loop somewhere.

Fooberman
  • 626
  • 5
  • 14
  • Thanks. I also had this in mind as a fallback, but I really hope there's a way to avoid this and rely on the constructor. Otherwise I have to do this for ALL the binary operators, which feels unnecessarily verbose. – David Thielke Feb 17 '12 at 07:55
  • @Foober Stackoverflow is a QA site. As such, answers are encouraged over opinions. 'my guess' and 'is simply broken' aren't really constructive. C++ is a welldefined language, and you'd do better to just explain how it works. – sehe Feb 17 '12 at 08:12
  • Part of the question was "how else would I overload the operator to have both of these work?" and I believe I have provided a working answer to that one. Sorry to bother. – Fooberman Feb 17 '12 at 08:25
  • @sehe `Stackoverflow is a QA site.` Does that mean we get to ask questions of the form: "Here is my code. Find the bugs in it for me" – Sam I am says Reinstate Monica Feb 04 '14 at 20:52
  • @SamIam I don't know. I've seen the strangest things. For example, I've been asked completely inane rhetorical questions, out of the blue, in comments on ancient answers by others. Fortunately, like you, I have the option to completely ignore this at my leisure. Ah. Freedom! – sehe Feb 04 '14 at 22:25
-1
2 * x;

is analogical equivalent to calling of,

int::operator*(const rantional<int>&)

2 is an int and it doesn't have operator * overloaded for const rational<int>&; thus you get compiler errors.

The correct way is to have:

rational<int> product2 = rational<int>(2) * x; // ok
iammilind
  • 68,093
  • 33
  • 169
  • 336
  • I understand that, but I thought that the compiler should be able to infer the constructor rational(int) from the argument. My question is more about why that inference isn't happening. – David Thielke Feb 17 '12 at 08:32
  • @DavidThielke, I have already explained the reason in my answer. Such implicit conversion don't happen. – iammilind Feb 17 '12 at 08:47