-1

About 50 minutes ago I started working on a project where I convert a string with roman numerals to an integer. I am having an issue where there is an "array overflow", and I've been troubleshooting for 30 minutes, and am getting kind of frustrated because I can't find a solution. The error that is shown is as follows

terminate called after throwing an instance of 'std::out_of_range'
what():  basic_string::at: __n (which is 3) >= this->size() (which is 3)

Here is my code:

class Solution {
  public:
    int romanToInt(string s) {
      int result = 0;
      char letters[7] = {'M','D','C','L','X','V','I'};
      int numbers[7] = {1000,500,100,50,10,5,1};
      for(int i=0; i<s.length(); i++) {
        if(i!=(s.length()-1)) {
          char *foo = std::find(std::begin(letters), std::end(letters), s.at(i));
          char *nfoo = std::find(std::begin(letters), std::end(letters), s.at(i+1));
          int num = numbers[std::distance(letters, foo)];
          int num2 = numbers[std::distance(letters, nfoo)];
          if(num<num2) {
            result+=(num2-num);
          }
          else {
            result+=num2;
          }
        }
        else {
          char *foo = std::find(std::begin(letters), std::end(letters), s.at(i));
          int num = numbers[std::distance(letters, foo)];
          result+=num;
        }
      }
      return result;
    }
};
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
TheBestMagician
  • 185
  • 1
  • 9
  • 1
    What do you want to happen in the last iteration when you do `s.at(i+1)`? – cigien Jul 22 '20 at 22:55
  • That is because if we have something like XL (40), we have to subtract X from L, since it comes before. That is a test for this. – TheBestMagician Jul 22 '20 at 22:56
  • 1
    But `s.at(i+1)` is just wrong, whatever you want to do. For any `s`, there is no char at that index. – cigien Jul 22 '20 at 22:57
  • Ohh, that might be it. Let me try. – TheBestMagician Jul 22 '20 at 22:58
  • Hmm... Now it doesn't throw an error, but the subtraction part doesn't work. – TheBestMagician Jul 22 '20 at 23:02
  • Thanks for the help! I still can't do it, so I edited the question. – TheBestMagician Jul 22 '20 at 23:09
  • 5
    @AnayAggarwal *About 50 minutes ago I started working on a project* -- You probably started writing code without first having a plan on paper. If you had a plan, then you write the code according to that plan. If the code doesn't work, you debug the code to see where the program diverges from your plan. Then you either make corrections to the code to follow the plan, or determine that the plan was wrong and make adjustments to the plan. [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) – PaulMcKenzie Jul 22 '20 at 23:32
  • You may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) – Andreas Wenzel Jul 23 '20 at 00:08
  • Wow, 50 minutes. So quick to jump onto StackOverflow. Please search the internet for "C++ roman numeral conversion example". Always search before posting to Stack Overflow. Although those examples are not yours, you can *compare* them to yours to see what you are doing wrong. – Thomas Matthews Jul 23 '20 at 00:51
  • 1
    BTW, you don't need to place all code into a class. You can make *free standing* functions. You example is a good case for not using a class. – Thomas Matthews Jul 23 '20 at 00:53
  • Please provide a [mre], are you still getting the same error now that you've edited your code? – Alan Birtles Jul 23 '20 at 06:02

1 Answers1

2

Your code can be cleaned up a bit:

int Roman_To_Decimal(const std::string& roman_number)
{
    static const char roman_letters[] = {'M','D','C','L','X','V','I'};
    static const int  values[] = {1000, 500, 100, 50, 10, 5, 1};
    const unsigned int string_length = roman_number.length();
    int decimal_value = 0;
    char const * const end_letter_iterator = &roman_letters[sizeof(roman_letters)];
    char const * const begin_letter_iterator = &roman_letters[0];
    for (unsigned int i = 0; i < string_length; ++i)
    {
       const char roman_digit = roman_number[i];
       char * position = std::find(begin_letter_iterator,
                                   end_letter_iterator,
                                   roman_digit);
       if (position != end_letter_iterator)
       {
           unsigned int index = position - begin_letter_iterator;
           const int digit_value = values[index];
           decimal_value += digit_value;
       }
    }
    return decimal_value;
}

The code above does not handle the case where a smaller Roman Numeral (letter) precedes a larger one, such as the cases of IX (9) and IV (4). This is an exercise for the OP or the reader.

Also, the above function is free standing, i.e. outside of a class, because not everything has to be in a class in C++.

Thomas Matthews
  • 56,849
  • 17
  • 98
  • 154