13 Replies - 4216 Views - Last Post: 07 April 2017 - 08:43 PM Rate Topic: -----

#1 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Incorrect Output for Roman Numerals to Integer conversion?

Posted 03 April 2017 - 10:13 PM

I am to write a program that converts a roman numeral into an integer to practice my class building skills. My professor requires that we use input/output text files, and my constraints are: Store the roman numeral as a string, convert the the roman numerals, store the positive integer, output either the roman numeral or integer value based on user choice. We are not allowed to go above the scope of the class, so I can't use any fancy functionality, fairly basic C++ stuff only.

//romanType.h, the specification file for the class romanType
#include <string>
#include <fstream>

using namespace std;

class romanType
{
public:
	void getRomanNumerals(ifstream& indata);
	//Get roman numerals from a file

	int valuesOfRomanNumerals(char roman);
	//Return the values of all possible roman numerals

	int convertRomansToIntegers();
	//Convert the roman numerals into integers

	void printRomanNumerals(ofstream& outdata);
	//Output the roman numerals to a file

	void printRomanIntegers(ofstream& outdata);
	//Output the positive integers to a file

private:
	string romanNumeral; //Variable to hold the roman numeral
	int romanInteger; //Variable to hold the positive integer
};


//Implementation file for the class romanType
void romanType::getRomanNumerals(ifstream& indata)
{
	indata >> romanNumeral;
}

int romanType::valuesOfRomanNumerals(char roman)
{
	switch(roman)
	{
	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;
	}
}

int romanType::convertRomansToIntegers()
{
	int index = 0,
		current = valuesOfRomanNumerals(romanNumeral[index]),
		next = valuesOfRomanNumerals(romanNumeral[index + 1]);

	for(index; index < romanNumeral.length(); index++)
	{
		if(next > current)
		{
			romanInteger = next - current;
		}
		else
		{
			romanInteger = current + next;
		}
		
		return romanInteger;
	}
}

void romanType::printRomanNumerals(ofstream& outdata)
{
	outdata << romanNumeral;
}

void romanType::printRomanIntegers(ofstream& outdata)
{
	outdata << romanInteger;
}


//User program using the class romanType
int main()
{
	ifstream infile("RomanInput.txt");
	ofstream outfile("RomanOutputNumerals.txt");
	ofstream outfile2("RomanOutputIntegers.txt");

	romanType romanObject;
	romanObject.getRomanNumerals(infile);
	romanObject.convertRomansToIntegers();

	romanObject.printRomanIntegers(outfile2);
	romanObject.printRomanNumerals(outfile);
	
	infile.close();
	outfile.close();
	outfile2.close();

	return 0;
}



It works fine as long as there are two roman numerals in the input file (IV and VI both return the proper value). However, when I do something like 'V', my output is -62. Similarly, when I use more than two numerals (VII, for example) it also returns incorrectly (6 instead of 7). There is something wrong with the conversion algorithm I'm using, and I just can't see it. Any help with this simple, frustrating problem is greatly appreciated!

Is This A Good Question/Topic? 0
  • +

Replies To: Incorrect Output for Roman Numerals to Integer conversion?

#2 integra94  Icon User is offline

  • New D.I.C Head

Reputation: 7
  • View blog
  • Posts: 26
  • Joined: 01-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 03 April 2017 - 10:27 PM

You don't iterate the string, you just save the first two values before the for loop.
Plus, you never really check if the string is shorter than two characters, so in a case of size() == 1, the value of 'next' is undefined (unhandled case of switch).
Was This Post Helpful? 1
  • +
  • -

#3 Xupicor  Icon User is offline

  • Nasal Demon
  • member icon

Reputation: 456
  • View blog
  • Posts: 1,179
  • Joined: 31-May 11

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 04 April 2017 - 07:23 AM

int romanType::valuesOfRomanNumerals(char roman)
{
	switch(roman)
	{
	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;
	}
    // <<< so... what value do I return if none of the above cases were matched??
}


Lack of some default value in that function is a problem - it means you get into undefined behavior land if the value of roman is not covered by the values you provided in your switch.
In other words, there's a possibility that a function that you promised should return a value of type int will "flow off the end" without actually returning any value. That's undefined behavior, and you shouldn't do it.
In practice (if the program doesn't blow up) there may be some value returned, but it may be unpredictable, and even if it looks like it is predictable - you really shouldn't rely on it.

As for your convertRomansToIntegers() - yeah... The algorithm doesn't quite work.

Maybe you should think about looping through your Roman numeral in reverse? Start with a sum and prev equal to 0, and in the loop assign your temp with a return value of valuesOfRomanNumerals(roman_numeral[index]) (did I mention that this function could do with a better name?) and check if temp is less than prev - if so... Well, you can figure out the rest, I bet.
Was This Post Helpful? 1
  • +
  • -

#4 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 04 April 2017 - 08:17 PM

I've taken the advice into account, and I'm almost there! I've changed the conversion algorithm as such:

int romanType::convert()
{
	//Testing for single character strings
	if(romanNumeral.length() == 1)
	{
		romanInteger = values(romanNumeral[0]);
		return romanInteger;
	}
	//For multicharacter strings
	else if(romanNumeral.length() > 1)
	{
		int index = romanNumeral.length() - 1;
		int sum = 0; //Total running calculation
		int counter = 0; //Integer to evaluate against the sum
		
		for(index; index >= 0; index--)
		{
			if(values(romanNumeral[index]) > counter) //Should always evaluate to True
			{
				sum = sum + values(romanNumeral[index]); //Starts the sum as the value of the leftmost character
				counter = values(romanNumeral[index - 1]); //Should set the counter to the next character
				
				if(counter < sum)
				{
					sum = sum - counter;
					counter = values(romanNumeral[index - 1]);
				}
				else if(counter >= sum)
				{
					sum = sum + counter;
					counter = values(romanNumeral[index - 1]);
				}
				else
				{
					return 0; //Should never reach this point
				}
			}
		}
		romanInteger = sum;
		return romanInteger;
	}
	else
	{
		return 0; //This point is only reached if there is no string to evaluate
	}
}



I thought by using the counter variable, I could iterate through the string and compare it to the value of the sum. However, I seem to be dropping some values along the way.

Input/Output:

I = 1
II = 2
III = 2 *Incorrect
V = 5
VI = 6
VII = 7
IV = 4
IIV = 4 (Not correct Roman Numerals, but for the purpose of this exercise it is fine) *Incorrect

Thanks for the help so far! You guys/gals are generally a much nicer lot than Stack Overflow and /r/learnprogramming.
Was This Post Helpful? 0
  • +
  • -

#5 integra94  Icon User is offline

  • New D.I.C Head

Reputation: 7
  • View blog
  • Posts: 26
  • Joined: 01-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 04 April 2017 - 10:30 PM

else if(counter >= sum) is unnecessary. Just leave it as 'else'.
There's also sum += counter, instead of sum = sum + counter, if you didn't know. 😅

Aside from nitpicking:
Look at what happens in case of 'III': In the first step you add set sum to one, then add the second I. In the second step, you again add the second I, so the sum == 3, and if you compare it to the third I, which is what you're doing, it merits a subtraction. 😕

You have to think of an algorithm to handle something like a 5 char long strings, and look at it step-by-step to see what it's doing. 😄
Was This Post Helpful? 1
  • +
  • -

#6 #define  Icon User is offline

  • Duke of Err
  • member icon

Reputation: 1849
  • View blog
  • Posts: 6,646
  • Joined: 19-February 09

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 04 April 2017 - 10:45 PM

If the string length is zero then the for loop would be able to handle that.

maxindex = length - 1 = 0 - 1 = -1.

So the initial value for index would be less than 0.

The for loop should also handle a string length of 1, but leave until the for loop works.


Perhaps instead of thinking about whether to add or subtract the character to the left, just think about adding or subtracting the current indexed character.

Do incorrect values such as IIV need to be solved correctly?
Was This Post Helpful? 1
  • +
  • -

#7 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 04 April 2017 - 11:40 PM

Quote

There's also sum += counter, instead of sum = sum + counter, if you didn't know. 😅

I knew there was a better way of doing this! I just couldn't remember quite how!

Quote

Look at what happens in case of 'III': In the first step you add set sum to one, then add the second I. In the second step, you again add the second I, so the sum == 3, and if you compare it to the third I, which is what you're doing, it merits a subtraction. 😕

That actually makes quite a bit of sense. Perhaps I should just set the sum value after doing the math, and not actually performing any calculation with it? Something like, evaluate the first and second I, store that as sum, evaluate the second and third I, and then add that to the sum? Leapfrog through the characters?

Quote

If the string length is zero then the for loop would be able to handle that.

I'm not very clear with good programming practice unfortunately, and my college career with it hasn't been particularly enlightening.

Quote

Do incorrect values such as IIV need to be solved correctly?

My professor just wants to see that we can iterate, assign, and sum the values of the letters. The project as a whole is just an exercise with C++ classes, he actually stated in the assignment notes how roman numerals generally work. A smaller numeral preceding a larger one warrants a subtraction, etc. He said for the purpose of his class, that's all he cares about.
Was This Post Helpful? 0
  • +
  • -

#8 integra94  Icon User is offline

  • New D.I.C Head

Reputation: 7
  • View blog
  • Posts: 26
  • Joined: 01-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 05 April 2017 - 01:58 AM

OK, clearly, the algorithm is currently your problem. You should let your program read it like you would read a roman numeral. This shows up first in a Google search:
1. Know the symbols and their values.
2. When one or more numeral is used to form a number, the value of each symbol is (generally) added together from left to right.
3. In some instances, a lower numeral placed in front of a larger numeral indicates that the lower numeral should be subtracted from the larger.

Personally, I would start by looking for the character of the highest value. Is there an M? If no, is there a D? If yes, subtract everything on the left and add everything on its right.
Perhaps even create something like const char * precedence = "MDCLXVI";
Was This Post Helpful? 1
  • +
  • -

#9 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 05 April 2017 - 11:41 PM

It is now working as intended. I admit I cheated it a little, and found an algorithm that worked online. Instead of calling it a day and receiving my grade, I was hoping someone here could help me understand why this conversion worked. I can read it, but I'm not sure I understand it.

int romanType::convert()
{
	int i, n, ans = 0, p = 0;
	n = romanNumeral.length() - 1;
	
	for(i = n; i >= 0; i--)
	{
		if(values(romanNumeral[i]) >= p)
			ans += values(romanNumeral[i]);
			
		else
		ans -= values(romanNumeral[i]);

	p = values(romanNumeral[i]);
	}
	romanInteger = ans;
	return romanInteger;
}



  • Declare i, n, ans, and p
  • Set ans and p to zero
  • Set n as the length of the string minus one (presumably to use as an index for the array)
  • Start the for loop, setting i equal to n; as long as i is greater than or equal to zero; decrement i by one
  • If the value found at the current index is greater than p(0), set ans to those values added together
  • Otherwise, set the value of ans to those values subtracted together
  • Every time the loop runs, change p to the current index value


My biggest questions are:
Why is it necessary to set i to n and use that as the indexing number? Why not just use n?

Why is p constantly changing values?
- I think I know this one, and this is partly why my conversion failed spectacularly. This conversion is using p as the 'next-in-line' character to evaluate to. So genius, so simple, why couldn't I think of that.

All-in-all, this has proven most worrying about my future in programming. :(/>
Was This Post Helpful? 0
  • +
  • -

#10 #define  Icon User is offline

  • Duke of Err
  • member icon

Reputation: 1849
  • View blog
  • Posts: 6,646
  • Joined: 19-February 09

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 06 April 2017 - 12:55 AM

Quote

Why is it necessary to set i to n and use that as the indexing number? Why not just use n?


It is not necessary. You could use n as the iteration variable as you suggest. Probably the thinking was that n is the calculated last index value which doesn't change in the call of the function. Since last index value doesn't change it might be clearer to leave it. The programmer might have not worked out the algorithm and kept n in case it was required. Personally, I think having the calculated value separate from the iterator breaks down the logical steps in the function hopefully making the code clearer.


Quote

Why is p constantly changing values?
- I think I know this one, and this is partly why my conversion failed spectacularly. This conversion is using p as the 'next-in-line' character to evaluate to. So genius, so simple, why couldn't I think of that.


Yes, p is the previous value which is held over for the next iteration.


Quote

All-in-all, this has proven most worrying about my future in programming.


There is more to programming than people might think. I don't know if you should worry, many beginners encounter problems. I remember not understanding things for days (weeks) and being flummoxed by simple things like misplaced semicolons, I know many others have had similar experiences. You are not just learning the language but tricks and patterns like the pattern/trick in the for loop using a variable to remember a value. You are also learning to understand computers, to design programs, minimize bugs and solve problems etc.
Was This Post Helpful? 1
  • +
  • -

#11 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 06 April 2017 - 01:11 AM

I appreciate all the help you guys have provided, and your words of encouragement. These are my first real forays into programming beyond simple python scripts, that didn't accomplish much more than printing out stuff. I'll be starting my next project soon, which is to take the source code that has a dateType class, and changing the constructor and one of the functions (setDate) to check if the date is valid. I also have to write a new function called isLeapYear, that will check all the years from 1900 on, to see if it is, in fact, a leap year.

That all should be quite the hoot, I can imagine I'll be around asking more questions.
Was This Post Helpful? 1
  • +
  • -

#12 Xupicor  Icon User is offline

  • Nasal Demon
  • member icon

Reputation: 456
  • View blog
  • Posts: 1,179
  • Joined: 31-May 11

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 07 April 2017 - 09:35 AM

Yup, I've said that a lot in the past - if you're just stepping into the world of programming - well, things are not going to be easy. There's a lot to learn beyond just the syntax of some particular language, it's the whole mindset that you need to grow into, not to mention ways of using variables, loops, etc, together in a "smart" way to accomplish goals.

There's quite a lot to take in. But... There's also a very special kind of joy and accomplishment when some particular baffling concept "clicks". Those moments are really, really satisfying. (Life being life, your newfound understanding is very soon put to the test and you get to see that it wasn't... uh... quite full. ; ) )

____
Ah, yes, as per usual I forgot my final point. : P
Press on, it'll get less baffling and frustrating, and more satisfying. (...and then it'll get more baffling again, but - you'll cross that bridge when you get there.)

This post has been edited by Xupicor: 07 April 2017 - 09:46 AM

Was This Post Helpful? 2
  • +
  • -

#13 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 5823
  • View blog
  • Posts: 19,821
  • Joined: 05-May 12

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 07 April 2017 - 10:12 AM

View PostXupicor, on 07 April 2017 - 12:35 PM, said:

But... There's also a very special kind of joy and accomplishment when some particular baffling concept "clicks". Those moments are really, really satisfying.


I call those moments "Hacker's High".
Was This Post Helpful? 1
  • +
  • -

#14 herringtonjc  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 12
  • Joined: 03-April 17

Re: Incorrect Output for Roman Numerals to Integer conversion?

Posted 07 April 2017 - 08:43 PM

I accomplished one of these joys recently. Last night I powered through the project to detect valid dates and leap years. I didn't get it right the first time, but I did get there in the end.

More surprisingly, I did it without asking anyone, or looking up code online. I do have a hefty text document that I was keeping notes in. Simple things like, which months have 31 days, which have 30, how to determine if a year is a leap year.

Then I wrote out the psuedocode in an English readable format, and made code for the words I was speaking. It turned out marvelously, and instead of something silly like creating an array of years that were leap years, I used the rules of leap years to write an algorithm to check whatever year was typed in.

It seems to work. It could probably be condensed; coming from a python background, people were always raving about making a fully function program in <10 lines of code, etc. For the purpose of my college class though, it'll do. I'm proud of it : D!

My next project is class composition, and after that, something to do with pointers. I do not enjoy memory management. We briefly covered those in an earlier class I took over C#, and I remember being baffled by it.
Was This Post Helpful? 1
  • +
  • -

Page 1 of 1