7

As part of my Computer Software Development degree, one of my labs consists of creating a calculator class template and a fraction class.

The problem is with my fraction class. My task is now to overload the plus operator to allow two fractions to be added together.

Fraction.cpp:

#include "Fraction.h"

const Fraction Fraction::operator+ (const Fraction &rhs) const
{
    return Fraction(_num * rhs.GetDen() + (rhs.GetNum() * _den), _den * rhs.GetDen());
}

Fraction.h

#pragma once

class Fraction
{
    public:
        Fraction(const int &num, const int &den) : _num(num), _den(den) {}
        ~Fraction(void) {}
        const int GetNum(void) { return _num; }
        const int GetDen(void) { return _den; }
        const Fraction operator+ (const Fraction &rhs) const;

    private:
        const int _num, _den;
};

Visual Studio complains that my fraction accessors cannot 'convert this pointer from const fraction to fraction &'. I'm completely baffled.

Bill the Lizard
  • 398,270
  • 210
  • 566
  • 880
Lee
  • 3,869
  • 12
  • 45
  • 65

5 Answers5

12

You need to qualify your accessors as const too:

int GetNum(void) const { return _num; }

BTW, qualifying the return type as const int does not really make sense, it will be an int anyway. Your compiler should emit a warning.

Gunther Piez
  • 29,760
  • 6
  • 71
  • 103
  • Aha! I knew it would be something to do with those pesky constants. Thanks a lot mate. – Lee May 11 '11 at 18:47
  • Is this same as `return (const int)_num;` ? Or does it have same effect? – atoMerz May 11 '11 at 18:50
  • @Lee Sounds like you accept drhirsch's answer, if so you should "accept" it formally. – DataGraham May 11 '11 at 18:53
  • 1
    @AtoMerZ No, the method must be declared as "const", essentially promising not to change the object, in order to be allowed to be called against a const object, and, similar to the note in drhirsch's answer that declaring the return type as "const" is pointless, it would be similarly pointless to cast it to "const", because the caller can always do whatever which their *copy* of the return value. – DataGraham May 11 '11 at 18:55
  • what adding const to the end of the declaration means is that it can be invoked by const instances of that class. It also means that member variables are treated as const while in the scope of that function, unless they have been declared as `mutable` – Dan F May 11 '11 at 18:56
  • @DataGraham I tried to many times to be greeted with "wait x minutes" :) – Lee May 11 '11 at 19:02
1

You GetNum and GetDen accessor methods are not declared as "const", so they cannot be called within the operator+ against const Fractions.

DataGraham
  • 1,625
  • 1
  • 16
  • 20
1

First question, why are you having the function return a "const Fraction" in the first place? That puts an arbitrary restriction on how your code can be used. It's quite clear that your Fraction class is already by and large implicitly const. No need to do so.

However, your member functions are non-const. That is, they don't tell the compiler that you won't be attempting to change state within the class itself. Please put the keyword "const" at the end of the member function declarations for all member functions which don't change state.

wheaties
  • 35,646
  • 15
  • 94
  • 131
  • This is just an example task. It serves no real purpose other than practice of following instructions from my lecturer and understanding what's going on. I can't tell you why I'm returning a const fraction. This definition was provided and I was to provide the body of it. – Lee May 11 '11 at 18:53
  • @Lee ok, that makes sense. I'd say that your instructor is encouraging bad coding practice with that. As long as you understand why or recognize that returning a "const object" like that from a function is generally not optimal design for flexibility you'll do well in your career. – wheaties May 11 '11 at 21:17
1

Your getters are not const:

    int GetNum() const { return _num; }
    int GetDen() const { return _den; }
         ///    ^^^^^^^^

You don't actually need these getters. Personally I would throw them away.
Note: That a class is a friend of itself and thus can access members of another instance.

const Fraction Fraction::operator+ (const Fraction &rhs) const
{
    return Fraction(_num * rhs._den+ (rhs._num * _den), _den * rhs._den);
}

Other Notes:

  • Also setting void as the parameter is not a good idea (this is C style).
  • const int has really no meaning just make it return int.
  • Please don't use underscore as the first character. Its legal but if you are not careful and don't know all the rules about underscore it is easy to trip up.
Martin York
  • 257,169
  • 86
  • 333
  • 562
  • I've been told it's good practice to use underscore for private members. – Lee May 11 '11 at 18:49
  • @Lee: Its not good practice it is just a personal style. It my opinion it gives you absolutely no benefit and you open yourself to a whole bunch of other potential problems. Now I have seen the prefix 'm_' work at least you can tell its a member. **Do you know the rules about using underscore in an identifier?** – Martin York May 11 '11 at 18:51
  • My lecturer uses m_ and I didn't understand what that mean until now, thanks for pointing that out. I'm afraid I'm ignorant about the underscore rules. – Lee May 11 '11 at 19:01
  • @Lee: Which is exactly why you should not use them at the beginning of identifiers. The rules are pretty obscure: [Summary here](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier/228797#228797). – Martin York May 11 '11 at 19:41
1
const int GetDen(void) { return _den; }

should (or might) be

int GetDen(void) const { return _den; }

The first one returns a const copy. A return by copy imply that the copy will be generated and provided to the caller function as a temporary object, therefore a const ("read-only") object. So returning by copy makes the copy read-only.

The second makes your function being callable even if the Fraction object is const - it's a "read-only" accessible function (you might say). It's probably what you wanted to write, and if not, it's better than without the const.

By the way, the void in parameters isn't useful in C++, you can avoid typing it and just say int GetNum() const{ return _num; }

Another detail : names starting by underscore caracter are reserved by the standard for standard implementations. It's not very dangerous to use them, but you never know. If you want to keep them, at least encapsulate all your code in namespace.

Klaim
  • 67,274
  • 36
  • 133
  • 188
  • I understand about the parameter issue. To be honest Visual Studio put the (void) in for me so I assumed it to be useful. On my own, from scratch, I'd normally leave them blank. Thanks for the useful info though :) – Lee May 11 '11 at 19:00
  • VS? Strange, it shouldn't even generate code for you. Are you using Visual Assist or some extensions? – Klaim May 11 '11 at 19:19
  • I'm using VS 2010 - Visual C++. When I "add" a class to my project. I know it's lazy, but it adds the header and cpp file for you and fills in the minimum definitions. – Lee May 11 '11 at 20:20