Fraction addition in C++

Question

So basically I am trying to do a fraction class. It will take in the fraction from user input and perform an addition. For example, I type in 1 5 and 1 7 , for the addition it will print out 12/35.
Here is my .h class:

#include <string>
#ifndef _FRACTION_H_
#define _FRACTION_H_

using namespace std;

class Fraction
{
    public:
        Fraction();
        Fraction(int n, int d);
        int getNumerator() const;
        int getDenominator() const;
        void display();
        string to_string();
        Fraction operator+(Fraction &second);
    private:
        int numerator;
        int denominator;
};

And this is my .cpp file:

#include "Fraction.h"
#include <string>
include <iostream>

using namespace std;

Fraction::Fraction(){}

Fraction::Fraction(int n, int d)
{
    this->numerator = n;
    this->denominator = d;
}

int Fraction::getNumerator() const
{
    return numerator;
}

int Fraction::getDenominator() const
{
    return denominator;
}

Fraction Fraction::operator+(Fraction &second)
{
    int n1 = getNumerator() * second.getDenominator();
    int n2 = second.getNumerator() * getDenominator();
    int d = getDenominator() * second.getDenominator();
    return Fraction(n1+n2, d);
}
string Fraction::to_string()
{
    return  (getNumerator() + "/" + getDenominator()) ;
}

And this is my main method:

bool get_input(Fraction &fract);

int main()
{
    Fraction fraction1, fraction2;
    if (((!get_input(fraction1)) || (!get_input(fraction2))))
    cout << "Invalid Input!" << endl;
    else 
    {
        // Test harness for Arithmetic Operator Overloading
        Fraction result = fraction1 + fraction2;
        cout << "Addition = " << result.to_string() << endl;
    }

    bool get_input(Fraction& fract)
    {
        int num, den;
        cout << "Enter numerator & denominator (separated by space)" << endl;
        cin >> num >> den;
        if (cin.fail())
        return false;
        Fraction f(num,den);
        fract = f;
        return true;
    }
}

It's managed to take in the user input. But however, it does not print out the result. Thanks in advance.

You're using a [reserved identifier](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). And please, for the sake of everyone, **never** put `using namespace ...;` in a header. Also, your `operator+` won't work with a temporary or const objects, which is unexpected. — chris, Jul 12 '13 at 08:18
Sorry what does that means? And how am I supposed to fix my code? I think something went wrong with my toString method — , Jul 12 '13 at 08:19
It does not print out the result? What _does_ it do? Have you stepped through it with your debugger? — Daniel Daranas, Jul 12 '13 at 08:19
Don't put `using namespace std` into header files. Use it in your cpp-files if you like, but under no circumstances in header files. — Oswald, Jul 12 '13 at 08:21
You're right, there is definitely something wrong with your `to_string` function. You're just advancing where the string literal begins, most likely past the end of it. Addition as concatenation does not work with C strings and integers, string literal or not. — chris, Jul 12 '13 at 08:23
I would say you *could* use it in main. You'd say `.cpp` too @Oswald? I got scared away from it period thanks to this site. I always use `using std::cout;` or sim now :) — ChiefTwoPencils, Jul 12 '13 at 08:23
But how should I fix it? I've been stucked here for an hour ago — , Jul 12 '13 at 08:24
@BobbyDigital, I didn't find it that hard to get into the habit of doing that after spending some time here. Now I'm really weighing out whether to use it when `std` is the only thing being used anyway, or as a definitive default, with everything else being qualified. With a little experience, you come to know which names are in use pretty quickly. — chris, Jul 12 '13 at 08:25
@Gwen concentrate in writing getNumeratorString() and getDenominatorString(), functions which should both return strings containing the string expression of their numeric values. Then you can easily use the combination of both. — Daniel Daranas, Jul 12 '13 at 08:25
@Gwen google `itoa` or `sprintf` and use one of them in your toString method — Cob013, Jul 12 '13 at 08:26
@Rob013, `itoa` is not a standard function. Try `std::to_string`. — chris, Jul 12 '13 at 08:27
@Gwen Look for it yourself, using Google. Hint: You can append " site:stackoverflow.com" at the end of your Google search, for better results. — Daniel Daranas, Jul 12 '13 at 08:28
isn't `std::to_string` only allowed in C++ 11 or am I wrong? — Cob013, Jul 12 '13 at 08:29
@DanielDaranas, The website filter can be put anywhere in the query :) — chris, Jul 12 '13 at 08:30
@chris: I agree. I have to say the first time I was warned about it I thought the notion of someone naming their stuff *exactly* like anything in `std` was hilarious. Since I've seen posts doing just that! — ChiefTwoPencils, Jul 12 '13 at 08:30
@Rob013, Correct. Prior to that, I would recommend `boost::lexical_cast`, or barring that, `std::ostringstream`. — chris, Jul 12 '13 at 08:30
@Rob013 There is no `itoa` function, either in C or in C++, and it's almost impossible to use `sprintf` safely. — James Kanze, Jul 12 '13 at 08:30
@chris Maybe I'm being dense, but I don't see the reserved identifier. (Or are you referring to `_FRACTION_H_`, which is undefined behavior.) — James Kanze, Jul 12 '13 at 08:33
@JamesKanze True, but I was only suggesting what to search over google. `std::ostringstream` is definely the best solution. — Cob013, Jul 12 '13 at 08:34
@JamesKanze, I was, and it's UB because it's a reserved identifier, or am I missing something (no matter how pedantic)? — chris, Jul 12 '13 at 08:35
Converting a number to a string and vice versa is a C++ FAQ, actually http://stackoverflow.com/q/5290089/96780 — Daniel Daranas, Jul 12 '13 at 08:37
@chris It's undefined behavior because the standard says so. But the wording has changed since I learned C++, and the standard does refer to them as "reserved". (When I learned C++, they were specified as undefined behavior in the description of identifiers, in the language, and not as a library issue.) — James Kanze, Jul 12 '13 at 08:41
@JamesKanze, That's interesting to know, thanks. I did look at the link again and see how the wording fit the UB in there. — chris, Jul 12 '13 at 08:42
@BobbyDigital You're even encouraged to do so, so that standard templates can pick them up. (Any time your class supports an optimized or non-throwing `swap`, for example, you should put a function named `swap` in the same namespace as the class.) — James Kanze, Jul 12 '13 at 08:43
@chris The standard takes the easy way out; if you violate any of the constraints on using the standard library, it's undefined behavior. I'm not sure, but I think that since the restriction is given in the library section, it only applies if you include at least one standard header. (When I was learning C++, the major C++ compiler generated C, and the constraint was part of the language so that the compiler could generate additional symbols without fear of conflict with user defined symbols. Not a library issue.) — James Kanze, Jul 12 '13 at 08:47
@JamesKanze, I like their style. Now if only a compiler was brave enough to blow up your house every time you invoked it :p — chris, Jul 12 '13 at 08:50
@chris For starters, it could refuse to compile the code (distinguishing between system headers and non-system headers). — James Kanze, Jul 12 '13 at 08:53

score 5 · Accepted Answer · answered Jul 12 '13 at 08:29

5

There maybe other problems, but the Fraction::to_string() function is clearly wrong: the types in the return expression are int, char const* and int. The result of adding these is a char const*, but given that the string literal is only two characters long, if the sum of the two int is greater than two, you have undefined behavior. You need to convert the int to strings first; the simplest way to do this is to use std::ostringstream:

std::string
Fraction::to_string()
{
    std::ostringstream results;
    results << numerator << '/' << denominator;
    return results.str();
}

Normally, one would expect a compiler error when you misuse types like this, but for historical reasons: string literals have type char const[], not std::string; char const[] converts almost everywhere to char const*; C++ supports adding integral values to pointers; and std::string has a constructor which does an implicit conversion from char const*.

answered Jul 12 '13 at 08:29

James Kanze

150,581
18
184
329

but it told me incomplete type is not allowed – Jul 12 '13 at 08:31
1

@Gwen, `#include `. – chris Jul 12 '13 at 08:32
I feel much relieved by the `s` suffix for string literals coming soon. – chris Jul 12 '13 at 08:34
So basically it's my method type is giving an error? Let's say I take in int but I store it as string. Am I explained in the correct way? – Jul 12 '13 at 08:35
2

@Gwen `int` and `std::string` (and `char const[]`) are distinct types. Each has rules concerning their use, and _normally_, if you violate the rules, you get an error from the compiler. You cannot assign an `int` to a `std::string`, for example. For historical reasons, however, C++ has a lot of implicit conversions, string literals don't have the type you'd expect, and the type they do have is more or less broken. So you don't get the compiler error. But that doesn't mean that the code is correct, nor that it does what you want. – James Kanze Jul 12 '13 at 08:50

score 1 · Answer 2 · answered Jul 12 '13 at 08:40

You use "+" operator between literals ("/") and int in your to_string method The compiler is trying to do some implicit conversion and it ends up with a point (int + int + char*, best guess is a char*), which is then transformed into a std::string using the correct constructor

Change your method to use stringstream (and you'll have finer control on formatting):

string Fraction::to_string()
{
    std::stringstream s;
    s << getNumerator() << "/" << getDenominator();
    return s.str();
}

reference : http://www.cplusplus.com/reference/sstream/stringstream/

Fraction addition in C++

2 Answers2