patterncppMinor
Integer to Roman Numeral converter
Viewed 0 times
romannumeralintegerconverter
Problem
I have implemented an Integer to Roman Numeral converter that converts any number in the range \$[1 \le x \le4999]\$ to its Roman number equivalent. Any comments on improving my code style or organization?
```
#include
#include
class Solution{
public:
Solution () : IntVal {1,5,10,50,100,500,1000} , roman {'I', 'V' , 'X', 'L' ,'C', 'D' ,'M'} {}
std::string intToRoman(int num);
private:
int IntVal[7];
char roman[7];
int fslt(int num);
std::string output;
void helper(int num);
void postProcess();
void str_replace( std::string &s, const std::string &search, const std::string &replace);
};
#include "roman.hpp"
int Solution::fslt(int num){
if (num>=50){
if (num=100 && num =500 && num =10) return 2;
else if (num>=5) return 1;
else return 0;
}
}
void Solution::helper (int num){
int div = fslt(num);
if (div == 0){
for (int i = 0; i <num ;i++){
output += roman[div];
}
return;
}
else {
int rn = fslt(num);
int numTimes = num/IntVal[rn];
for (int i=0;i < numTimes; i++){
output += roman[rn];
}
helper(num-numTimes*IntVal[rn]);
}
}
void Solution::postProcess(){
str_replace(output, "DCCCC", "CM");
str_replace(output, "CCCC", "CD");
str_replace(output, "LXXXX", "XC");
str_replace(output, "XXXX", "XL");
str_replace(output, "VIIII", "IX");
str_replace(output, "IIII", "IV");
}
std::string Solution::intToRoman(int num){
Solution::helper(num);
Solution::postProcess();
return output;
}
void Solution::str_replace( std::string &s, const std::string &search, const std::string &replace)
{
for( size_t pos = 0; ; pos += replace.length() )
{
pos = s.find( search, pos );
if( pos == std::string::npos ) break;
s.erase( pos, search.length() );
s.insert( pos, replace );
}
}
int main(){
Solution mySol;
std::cout <<
```
#include
#include
class Solution{
public:
Solution () : IntVal {1,5,10,50,100,500,1000} , roman {'I', 'V' , 'X', 'L' ,'C', 'D' ,'M'} {}
std::string intToRoman(int num);
private:
int IntVal[7];
char roman[7];
int fslt(int num);
std::string output;
void helper(int num);
void postProcess();
void str_replace( std::string &s, const std::string &search, const std::string &replace);
};
#include "roman.hpp"
int Solution::fslt(int num){
if (num>=50){
if (num=100 && num =500 && num =10) return 2;
else if (num>=5) return 1;
else return 0;
}
}
void Solution::helper (int num){
int div = fslt(num);
if (div == 0){
for (int i = 0; i <num ;i++){
output += roman[div];
}
return;
}
else {
int rn = fslt(num);
int numTimes = num/IntVal[rn];
for (int i=0;i < numTimes; i++){
output += roman[rn];
}
helper(num-numTimes*IntVal[rn]);
}
}
void Solution::postProcess(){
str_replace(output, "DCCCC", "CM");
str_replace(output, "CCCC", "CD");
str_replace(output, "LXXXX", "XC");
str_replace(output, "XXXX", "XL");
str_replace(output, "VIIII", "IX");
str_replace(output, "IIII", "IV");
}
std::string Solution::intToRoman(int num){
Solution::helper(num);
Solution::postProcess();
return output;
}
void Solution::str_replace( std::string &s, const std::string &search, const std::string &replace)
{
for( size_t pos = 0; ; pos += replace.length() )
{
pos = s.find( search, pos );
if( pos == std::string::npos ) break;
s.erase( pos, search.length() );
s.insert( pos, replace );
}
}
int main(){
Solution mySol;
std::cout <<
Solution
Helper function too complex
Your
Your
helper function and the fslt function seem too complex for what they do (helper is even recursive!). All you are really doing is adding roman numerals starting at the highest value first. So something like this would suffice:#define DIM(arr) (sizeof(arr)/sizeof(arr[0]))
void Solution::helper (int num){
for (int i = DIM(IntVal)-1; i >= 0; i--) {
while (num >= IntVal[i]) {
output += roman[i];
num -= IntVal[i];
}
}
}Code Snippets
#define DIM(arr) (sizeof(arr)/sizeof(arr[0]))
void Solution::helper (int num){
for (int i = DIM(IntVal)-1; i >= 0; i--) {
while (num >= IntVal[i]) {
output += roman[i];
num -= IntVal[i];
}
}
}Context
StackExchange Code Review Q#101143, answer score: 4
Revisions (0)
No revisions yet.