1. Convert string_iterator to string

I have a piece of code that is converting Roman Numerals to int.

Code:
int RomanToInt(const string & Roms){
if (Roms.size() == 0)
return 0;

unordered_map<string,int> Rom_Map = {{"I",1},
{"V",5},
{"X",10},
{"L",50},
{"C",100},
{"D",500},
{"M",1000},
{"IV",4},
{"IX",9},
{"XL",40},
{"XC",90},
{"CD",400},
{"CM",900}};
auto first = Roms.begin();
auto second = next(first);
auto third = next(second);

auto res = 0;

while (first != Roms.end()){
auto cChar = string{*first};

string dChar = "";
if (second != Roms.end())
dChar = string{*first} + string{*second};

if (Rom_Map.find(dChar) != Rom_Map.end()){
res += Rom_Map.at(dChar);
first = third;
}
else if (Rom_Map.find(cChar) != Rom_Map.end()){
res += Rom_Map.at(cChar);
first = second;
}

second = next(first);
third = next(second);
}

return res;
}

}
Since I'm declaring the first type as string in my unordered_map, I have to convert the string_iterator to std::string.This looks kind of inefficient. Is there a more efficient way to do this?

Code:
// different way to do this?
auto cChar = string{*first};
dChar = string{*first} + string{*second}; 2. Originally Posted by nimitzhunter
Since I'm declaring the first type as string in my unordered_map, I have to convert the string_iterator to std::string.This looks kind of inefficient. Is there a more efficient way to do this?
I do not think you can avoid it: you are iterating over characters, but searching the map for strings.

By the way, although it works for me (and presumably works for you), I believe that next(first) and next(second) actually results in undefined behaviour if Roms.begin() and next(first) resulted in a one-past-the-end iterator, respectively. I suggest re-arranging the code to avoid this, e.g.,
Code:
int RomanToInt(const string & Roms)
{
if (Roms.empty())
{
return 0;
}

static const auto Rom_Map = unordered_map<string, int>{{"I", 1},
{"V", 5},
{"X", 10},
{"L", 50},
{"C", 100},
{"D", 500},
{"M", 1000},
{"IV", 4},
{"IX", 9},
{"XL", 40},
{"XC", 90},
{"CD", 400},
{"CM", 900}};

auto result = 0;

auto first = Roms.begin();
while (first != Roms.end())
{
auto valid = false;

auto second = next(first);
if (second != Roms.end())
{
auto found = Rom_Map.find(string{*first} + *second);
if (found != Rom_Map.end())
{
valid = true;
result += found->second;
first = next(second);
}
}

if (!valid)
{
auto found = Rom_Map.find(string{*first});
if (found != Rom_Map.end())
{
valid = true;
result += found->second;
first = second;
}
}

if (!valid)
{
// Throw an exception?
break;
}
}

return result;
} Popular pages Recent additions 