6

I have a rational number class is made up of two integers: num, the nominator, and den, the denominator.

The following operator is supposed to read the rational number from a stream.

istream& operator >> (istream& Is, rational& r) {
  char c; //Test char.
   double n; //Could be either the numerator of the fraction or the antiperiod of the repeating decimal number.
    Is >> n;
    int i = 0;
    for (; n*10-pow(10, i+1) < 1 && int(n) != 0; i++) {
        n *= 10;
    }
    for (; int(n*10) % 10; i++) {
        n *= 10;
    }
    n /= pow(10, i);
    if (i == 0) {
        r.num = n;
        Is >> ws;
        c = Is.peek();
        if (c == '/') {
            c = Is.get();
            Is >> r.den;
        } else {
            r.den = 1;
        }
        r.normalize(); //This function normalizes the fraction.
    } else {
        Is >> ws;
        c = Is.peek();
        if (c == 'p' || c == 'P') {
            int p; //Period of the repeating decimal number.
            c = Is.get();
            Is >> p;
            vector<int> a = genFrac(n, p); //This function returns the fraction which express the decimal repeating number. It returns a "vector<int>" with the nominator at index 1 e denominator at index 0.
            r.num = a[1];
            r.den = a[0];
        } else {
            i = 0;
            for (; n*10-pow(10, i+1) < 1 && int(n) != 0; i++) {
                n *= 10;
            }
            for (; int(n*10) % 10 != 0; i++) {
                n *= 10;
            }
            int pot10 = pow(10, i);
            r.num = n;
            r.den = pot10;
        }
        r.normalize();
    }
    return Is;
}

I wrote this code to implement the input of my "rational" class. I modified it from the one written in my C++ book in order to make possible the input of decimal numbers, including repeating ones.

It should be able to handle these types of input:

  • 9/8
  • 9
  • 9.87
  • 1.p3 (= 1.3333333333)

But it doesn't work, not even the part I copied from the book.

Can anyone help me?

lytenyn
  • 819
  • 5
  • 21
fpiro07
  • 847
  • 1
  • 13
  • 18
  • 10
    Are you getting compilation errors, runtime errors, unexpected output? Details on how it's failing will be helpful. – RonaldBarzell Dec 05 '12 at 14:14
  • I don't get errors, it's just that if I for example try the d) type of input it stops at the "c = is.peek()" because it waits for another character but I want the "p" to be that character... I don't know if you understood it's quite complicated to explain. It definitely is a bug in the algorithm but I can't find it. – fpiro07 Dec 05 '12 at 14:21
  • 1
    Why don't you read the whole stream first and then analyze it? – Rafa Dec 05 '12 at 14:52
  • Yeah that could do, how can I do that? With "getline"? – fpiro07 Dec 05 '12 at 14:59

2 Answers2

5

I think I'd write this somewhat differently1.

Unless you really need to do otherwise, I'd start by reading an entire "chunk" of input (i.e., all the characters up to the next white space), then sort out how that's supposed to represent a number, and call a separate function for each possible representation:

std::istream &operator>>(std::istream &is, rational &r) {
    std::string temp;

    Is >> temp;
    if (temp.find('/') != std::string::npos)
        r = cvt_fraction(temp, Is);
    else if (temp.find_first_of("pP") != std::string::npos)
        r = cvt_repeating(temp, Is);
    else if (temp.find('.') != std::string::npos)
        r = cvt_float(temp, Is);
    else
        r = cvt_int(temp, Is);
    return Is;
}

I've passed the istream to each for two reasons: first, so if they find garbage in the input, they can set the stream's fail bit. Second, so if they really do need to read more input, they can (but I'd be a little surprised if that's ever really needed).

It seems to me that each of those conversion functions should be fairly trivial: if I'm starting from the fact that a string should be digits "/" digits or `digits "p" digits", doing a conversion is generally going to be pretty simple -- specifically, simple enough that I think just about anybody can probably glance at the code and sort out what each piece is supposed to do.


  1. I honestly don't mean to be nasty, but if I was maintaining code, and ran across your operator>>, I would have one of two possible reactions: if it apparently had a bug, replace it immediately. Otherwise, put it on the "technical debt" list, and replace it as soon as possible. The simple fact is that as it stands right now, it takes a fair amount of study to even be sure what input formats are supposed to be supported, not to mention which part of the code handles each, or how the whole thing is supposed to work together to produce a meaningful result.
Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • Why is it necessary to pass also "is" as a parameter to the functions? Isn't it enough to pass only the string? – fpiro07 Dec 08 '12 at 08:55
  • @fpiro07: As noted in the paragraph immediately following the code, primarily so they can set the stream's failbit in case of bad input. – Jerry Coffin Dec 08 '12 at 12:17
1

The issue mentioned in the comment (the p not appearing in the c=is.peek() statement) comes from the fact that the p is actually stored in ws (it is stored there in is >> ws).

The above code also contains no mention of ws, but I assume it is a char.

lytenyn
  • 819
  • 5
  • 21
  • Ok thanks so what can I do to solve the problem? Can I swap the "c = 'p' " with a "ws = 'p' "? – fpiro07 Dec 05 '12 at 14:54
  • I think you are somehow misunderstanding the meaning of `Is >> ws`. This will read a `char` from a string and is probably wrong in both branches of the `if`. Try to remove both `Is >> ws` and the `Is >> p`. – lytenyn Dec 05 '12 at 14:58
  • And what about the "is.peek()"? Is it still useful or not? – fpiro07 Dec 05 '12 at 15:02
  • Sorry, I got confused myself. `Is >> p` is still required. Of course you also need the `is.peek()`, otherwise the variable `c` will not be initialized! – lytenyn Dec 05 '12 at 15:15
  • I'm sorry I didn't actually understand your solution. Could you please explain it again? – fpiro07 Dec 06 '12 at 17:08
  • Honestly, the above answer is much better. Just re-structure your code as suggested. I really should have suggested a re-write myself. – lytenyn Dec 07 '12 at 08:04