C++

Forum

 
Need help fixing my converter

Tduck (67)
This is a roman numeral to integer conversion program; I've been desparately trying to fix this program, and I've gotten so close. Example, my input is CDLXII and I get 461 instead of 462. I can't figure out how to debug this, and I'm tapped on trying to understand what to do here. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
 
#include <iostream>
#include <string>

using namespace std;

//Convert roman numerals to decimal form.  
int main()
{
    //Take roman numeral input
    string romanInput = "";
    cout << "Enter roman numeral in all caps:  ";
    cin >> romanInput;
    //romanInput needs to be split into an array of chars.
    int stringLength = romanInput.length();
    int intTranslation[stringLength];
    char romanChar[stringLength];
    for(int i = 0; i < stringLength; i++)
    {
        romanChar[i] = romanInput[i];
        switch(romanChar[i])
        {
           case 'I':
           intTranslation[i] = 1;
           break;
           case 'V':
           intTranslation[i] = 5;
           break;
           case 'X':
           intTranslation[i] = 10;
           break;
           case 'L':
           intTranslation[i] = 50;
           break;
           case 'C':
           intTranslation[i] = 100;
           break;
           case 'D':
           intTranslation[i] = 500;
           break;
           case 'M':
           intTranslation[i] = 1000;
           break;
           default:
           break;
        }
    }
    int total = 0;
    if(stringLength == 1)
    {
        total = total + intTranslation[0];
    }
    if(stringLength == 2)
    {
        if(intTranslation[0] < intTranslation[1])
        {
            total = intTranslation[1] - intTranslation[0];
        }
        else { total = intTranslation[0] + intTranslation[1]; 
        }
    }
    else if(stringLength > 2)
    {
        for(int i = 0; i < stringLength; i++)
        {
            if(intTranslation[i] < intTranslation[i+1] and stringLength > (i+1) ) 
            {
                total = total + (intTranslation[i+1] - intTranslation[i]); 
                intTranslation[i] = 0;
                i = i+1;
                
            }
            else if(intTranslation[i] > intTranslation[i+1] or intTranslation[i] == intTranslation[i+1])
            {
                total = total + intTranslation[i];
            }
        }
    }
    //if string length is greater than 3, use following comments for precode.  
   cout << "Your roman numeral in decimal form:  " << total << flush;
    //add intTranslation[i]'s up for total.  
    //if the translation is less than the next, it has to be the next - the previous = both of them added together.  
}
Last edited on
mbozzi (3932)
You're over-complicating things.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
 
#include <iostream>

constexpr int value(char c) {
  switch (c) {
  case 'I': return 1;
  case 'V': return 5;
  case 'X': return 10;
  case 'L': return 50;
  case 'C': return 100;
  case 'D': return 500;
  case 'M': return 1000;

  default: return 0;
  }
}

int main() {
  std::string roman = "CDLXII"; // 462

  int max = 0, acc = 0;
  for (auto it = roman.rbegin(); it != roman.rend(); ++it) {
    int const v = value(*it);
    max = max > v ? max : v;
    acc += (v < max ? -v : v);
  }

  std::cout << acc;
}
Last edited on
Tduck (67)
This code has errors and I've not ever seen a lot of these things, like rend(), rbegin(), auto, or ?
I'm sure there are simpler ways to do it that I'm unaware of, but I can't do them unless I know what the things are.
closed account (E0p9LyTq)
This code has errors

No errors if using a newer compiler. Using Visual Studio 2017 with mbozzi's code compiled without a problem.

What compiler are you using, Tduck?

I've not ever seen a lot of these things, like rend(), rbegin(), auto, or ?
I'm sure there are simpler ways to do it that I'm unaware of, but I can't do them unless I know what the things are.

Then you should spend some time looking at what these old and new features of C++ do.

Some of the items are C++98, some C++11 and later.

http://www.cplusplus.com/reference/string/string/rbegin/
http://www.cplusplus.com/reference/string/string/rend/

The auto specifier is especially useful.
https://en.cppreference.com/w/cpp/language/auto
Last edited on
JLBorges (13770)
See if you can understand this (equivalent) code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
 
#include <iostream>

int value_of( char c ) { // value of roman digit c

  switch (c) {

      case 'I': case 'i' : return 1;
      case 'V': case 'v' : return 5;
      case 'X': case 'x' : return 10;
      case 'L': case 'l' : return 50;
      case 'C': case 'c' : return 100;
      case 'D': case 'd' : return 500;
      case 'M': case 'm' : return 1000;

      default: return 0;
  }
}

int main() {

  // we assume that the roman string is well formed
  const std::string roman = "CDLXII"; // 462
  const int n = roman.size() ;

  int max_digit_seen_so_far = 0 ;
  int value_of_roman = 0;

  for( int i = n-1 ; i >= 0 ; --i ) { // iterate backwards, from right to left

    const int this_digit = value_of( roman[i] ) ; // value of this digit

    if( max_digit_seen_so_far < this_digit ) max_digit_seen_so_far = this_digit ;

    // we assume that there would not be a signed integer overflow
    if( this_digit < max_digit_seen_so_far ) value_of_roman -= this_digit ;
    else value_of_roman += this_digit ;
  }

  std::cout << "roman: " << roman << "   value: " << value_of_roman << '\n' ;
}

Tduck (67)
My code compared to the above doesn't seem terribly complex, and yeah, besides the reason you're using i-- instead of i++ I understand. I appreciate the help, and I did learn about rend and auto, but I still can't figure out how to fix the code I wrote.

I can't see what I'm doing wrong if people have done the work for me.
Edit: Also I super simplified my code in my freetime, specifically realizing the initial if statement that evaluated the string length was completely unnecessary, and some cleaning up.
Last edited on
jonnin (11435)
ill help you debug yours; your attitude is commendable. Learn from both, study their code later, but lets fix yours first.

Add print statements, is how I do it. Print the letter, what you got from translation, and total, for every iteration of that for loop. It will tell you why you didn't get the right answer. Try that, and if you don't understand how to proceed, Ill walk you thru it. You can do this with your debugger and you should learn that process, but for simple things a spew of print statements is a lot faster than stepping through the code (but less information available).

c++ does not support variable array sizes (int x; cin >> x; int array[x] is illegal). Most but not all compilers support it if not put into strict mode.


most programmers are going to use symbolic comparisons.
&& -> and
|| -> or
! -> not
you almost never see the textual ones in c++. Its not an error.

-------Do not read this part yet ------------













I added the print statements and behold, there is nothing printed when i is one. printing the loop variable (always, always print these) shows 0,2,3,4 only. You have a logic problem; the two if statements are both excluding 1.
this fixes it:
else // if(intTranslation[i] > intTranslation[i+1] or intTranslation[i] == intTranslation[i+1])

but that is only one test case. youll have to see if you need to check something there and if so, what the test should be to get it right.
Last edited on
Tduck (67)
Thank you so much jonnin, your debugging methods have helped me out a great deal. Now I just have to figure out how to use fstream, haha. I'll make sure to use the print method for programs with loops like this.

Edit: here is the code, by the way. I ended up having trouble with CDLXIII once I fixed CDLXII and so I debugged that with the same method. Though I'm questionning why cpp.sh won't compile it but other compilers will, cpp.sh does that a lot or just doesn't work right sometimes though so I won't question it for the moment.

edit2: Yep now cpp.sh runs it, just cpp.sh being weird. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
 
#include <iostream>
#include <string>

using namespace std;

//Convert roman numerals to decimal form.  
int main()
{
    //Take roman numeral input
    string romanInput = "";
    cout << "Enter roman numeral in all caps:  ";
    cin >> romanInput;
    //romanInput needs to be split into an array of chars.
    int stringLength = romanInput.length();
    int intTranslation[stringLength];
    char romanChar[stringLength];
    for(int i = 0; i < stringLength; i++)
    {
        romanChar[i] = romanInput[i];
        switch(romanChar[i])
        {
           case 'I':
           intTranslation[i] = 1;
           break;
           case 'V':
           intTranslation[i] = 5;
           break;
           case 'X':
           intTranslation[i] = 10;
           break;
           case 'L':
           intTranslation[i] = 50;
           break;
           case 'C':
           intTranslation[i] = 100;
           break;
           case 'D':
           intTranslation[i] = 500;
           break;
           case 'M':
           intTranslation[i] = 1000;
           break;
           default:
           break;
        }
    }
    int total = 0;
    if(stringLength == 1)
    {
        total = total + intTranslation[0];
    }
    if(stringLength == 2)
    {
        if(intTranslation[0] < intTranslation[1])
        {
            total = intTranslation[1] - intTranslation[0];
        }
        else { total = intTranslation[0] + intTranslation[1]; 
        }
    }
    else if(stringLength > 2)
    {
        for(int i = 0; i < stringLength; i++)
        {
            if(intTranslation[i] < intTranslation[i+1] and stringLength > (i+1) ) 
            {
                total = total + (intTranslation[i+1] - intTranslation[i]); 
                intTranslation[i] = 0;
                i = i+1;
                cout << "temp total:  " << total << endl;
            }
            else if(intTranslation[i] > intTranslation[i+1] or intTranslation[i] == intTranslation[i+1])
            {
                total = total + intTranslation[i];
                cout << "temp total:  " << total << endl;
                if(intTranslation[i] == intTranslation[i+1]) { total = total + intTranslation[i];}
                if(intTranslation[i] == intTranslation[i+2]) { total = total - (2 * intTranslation[i]);}
            }
        }
    }
    cout << "Your roman numeral in decimal form:  " << total << flush;
}
Last edited on
jonnin (11435)
cpp.sh sounds like a unix shell script that someone provided to you. That means it is probably a text file, so you can type 'cat cpp.sh' to see what it is.



Tduck (67)
So that's why it sometimes outputs random text like meow meow woof meow. I looked it up and I found a bunch of example programs tied to cpp.sh

Edit: I'm getting an uncomfortable sexual vibe from this example program which uses random words/adjectives madlib style.

"Hi, I'm Remy and I want to be your tasty nasty nurse

I've been called a favorite couch by Mr. Potato Head but you should come see for yourself

I'm available at my van down by the fridge

I'd love to wash your wet teeth while you lick my special nostrils until we detach under downtown

Iron me soon!

Love The Etch-a-Sketcher"
Last edited on
mbozzi (3932)
cpp.sh is the online compiler integrated into this forum:
http://cpp.sh/
The little gear icon that appears when a code snippet (in code tags) has a main function is a link.
Last edited on
Ganado (6808)
jonnin, I'm legitimately surprised you didn't know what cpp.sh was :D
But hey, now you know. https://www.youtube.com/watch?v=pele5vptVgc
jonnin (11435)
Ah, I didn't know its name :)
I use it of course.
Topic archived. No new replies allowed.