-3

I'm a Python kiddie looking to sharpen my CPP skills, so I did a Leetcode problem to convert integers into roman numerals. Here's the original code. Notice the lines I've marked as incorrect.

    string intToRoman(int num) {
        int values[] = {1000, 500, 100, 50, 10, 5, 1};
        char keys[] = {'M', 'D', 'C', 'L', 'X', 'V', 'I'};

        string result = "";

        int current_val, digits;

        for (int i = 0; i < 7; i++) {
            current_val = values[i];
            digits = num / current_val;

            if (digits == 4 && keys[i] != 'M') { // INCORRECT LINE #1
                if (result.back() == keys[i-1]) { // INCORRECT LINE #2
                    result[result.length() - 1] = keys[i];
                    result += keys[i-2];
                } else { // 4
                    result += keys[i];
                    result += keys[i-1];
                }
            } else {
                string addon(digits, keys[i]);
                result += addon;
            }
            num %= values[i];
        }
        return result;
    }

VScode ran this without any issues whatsoever, but when I plugged it into Leetcode, I got the following exception:

Line 1065: Char 9: runtime error: addition of unsigned offset to 0x7ffc4087c310 overflowed to 0x7ffc4087c30f (basic_string.h)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/basic_string.h:1074:9

After doing some research, I concluded that C++ really does not like negative indices, even when they aren't possible. So I added some checks to the incorrect lines:

if (i >= 1 && digits == 4 && keys[i] != 'M') { // CORRECT LINE #1
                if (i >= 2 && result.length() >= 1 && result.back() == keys[i-1]) { // CORRECT LINE #2

The code works now, but it left me very confused. Why did my code work with one interpreter and not another? Obviously there's something I'm missing regarding how C++ compiles code. In practice, the numbers involved should never be negative. So why do I get a runtime error?

Benjamin Gilbert
  • 7
  • 6
    C++ doesn't has interpreters, it has compilers (which don't execute the code themselves, they convert it to something the processor can execute directly). VSC isn't a compiler, it's irrevelant that you use it. A compiler is most likely either GCC, Clang, or MSVC. – HolyBlackCat Aug 26 '23 at 07:33
  • 6
    Leetcode and other so-called "competition" sites are the worst place to learn anything about programming. They already assumes you're very good at the selected language, and programming in general. If you want to "sharpen" your C++ knowledge, invest in [some good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and take classes. All that seems to be taught by such sites are bad habits, bad code, and too often even *invalid* code. – Some programmer dude Aug 26 '23 at 07:34
  • 2
    C++ has a concept of undefined behavior: some errors can have unpredictable effects. Such as going out of bounds. Leetcode enabled optional bounds tests, so going out of bounds caused a runtime error. You didn't enable this check, so your code happened to silently appear to work. If you tell us your OS and compiler, we can tell how to enable more of those checks. – HolyBlackCat Aug 26 '23 at 07:35
  • 1
    As a hint: If `i == 0`, which it will be the first iteration of the loop, what is `i - 1`? Is `i - 1` a valid index in the case of `i == 0`? How about `i - 2`? – Some programmer dude Aug 26 '23 at 07:36
  • @Benjamin - Likewise for `result.back()`, which is only valid when `result` holds at least one value. So on line #2 you risk shooting yourself in *both* feet! – BoP Aug 26 '23 at 08:35
  • @Benjamin - Also, unlike in ancient C, you don't have to declare all the local variables at the start of a function. You can wait until they have a value, and then often they can be made `const`. Like using `const int digits = num / current_val;` *inside* the loop. – BoP Aug 26 '23 at 08:39
  • I suggest testing by outputting every Roman numeral between 1 and 3999. When I tried this with your "incorrect" code, it hung up on 4, with the error: "back() called on empty string". Here is the code I used: `for (int i{1}; i < 4000; ++i) {std::cout << std::format(" {:4} {} \n", i, Solution{}.intToRoman(i)); }` – tbxfreeware Aug 26 '23 at 08:52
  • Accessing an array out of bounds, is classic *undefined behaviour*. See the duplicate for an explanation of the important C++ concept. – john Aug 26 '23 at 09:12
  • _Pro tip:_ `std::string` has two versions of member function `back()`. One returns `char &`, which is read as "reference to char." The other returns `char const&`, i.e., "reference to const char." For strings that are `const`, the compiler uses the second one, but for other strings, it uses the first. The cool thing about this is that _the first one can receive assignment._ So, if you think it makes the program more readable, `result[result.length() - 1] = keys[i];` can be changed to `result.back() = keys[i];` . – tbxfreeware Aug 26 '23 at 09:14
  • A fun challenge that most C++ students take on sooner or later is to code the reverse conversion: `int romanToInt(std::string roman);`. – tbxfreeware Aug 26 '23 at 09:22

0 Answers0