[C#] Need help for converting Roman Numerals to Decimal/Integer

[Gil37]

Hi to all members and admins here.... and here I am posting a the same question

 

I already create a thread but the admin close it....

 

I have this problem on C# and I can't solve it. Please help....

 

i use simplezz code: https://www.neowin.net/forum/topic/1228967-c-convert-roman-numerals-to-decimalinteger/?view=findpost&p=596572777

 

and give me error

 

here are the error of simplezz code:

 

1. A namespace does not directly contain members such as fields or methods line 73 file roman.cs

2. Expected class, delegate, enum, interface, or struct line 83 file roman.cs

3. Expected class, delegate, enum, interface, or struct line 122 file roman.cs

4. Expected class, delegate, enum, interface, or struct line 127 file roman.cs

5. Expected class, delegate, enum, interface, or struct line 132 file roman.cs

6. Expected class, delegate, enum, interface, or struct  line 133 file roman.cs

7. Type or namespace definition, or end-of-file expected line 141 file roman.cs

 

 

and now this is my own code... some of the codes are from the internet.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            Roman rm = new Roman();
            Console.WriteLine("enter roman: ");

            string rom = Console.ReadLine();
            rm.RomanNumber = rom;
            int dc;
            int i;
            try
            {
                for ( i = 0; i < 10; i++)
                {
                    switch (rm.RomanNumber[i])
                    {
                        case 'M':
                        case 'm':
                            dc = 1000;
                           Console.WriteLine(dc);
                            break;
                        case 'D':
                        case 'd':
                            dc = 500;
                           Console.WriteLine(dc);
                            break;
                        case 'C':
                        case 'c':
                            dc = 100;
                           Console.WriteLine(dc);
                            break;
                        case 'L':
                        case 'l':
                            dc = 50;
                           Console.WriteLine(dc);
                            break;
                        case 'X':
                        case 'x':
                            dc = 10;
                           Console.WriteLine(dc);
                            break;
                        case 'V':
                        case 'v':
                            dc = 5;
                            Console.WriteLine(dc);
                            break;
                        case 'I':
                        case 'i':
                            dc = 1;
                            Console.WriteLine(dc);
                            break;
                        default:
                            break;
                        //Console.Write("error
                    }

                    //return dc;

                }
                if (rom[i] < rom[i])
                {
                    dc = rom[i] - rom[i];
                    Console.WriteLine("dc");
                }
                else if(rom[i] >= rom[i])
                {
                    dc=rom[i]+rom[i];
                    Console.WriteLine("dc");
                }
               

            }
            catch (IndexOutOfRangeException e) { }
        }
    }
}
 

and here is the output

 

Enter roman:

mmxiv

1000

1000

10

1

5

 

it means the result is 2016 if they add but the expected output it must be 2014

 

 

btw where is the link for all the BB Codes? usually on some forums is located at the bottom but on this forum. I can't find it

 

Note: please don't close this thread. I'm only using my friend laptop to post my question. I need to reply if ever someone reply to this thread

[Andre S. Veteran]

You'll learn nothing and defeat the purpose of your homework by copying other code found on the internet or elsewhere. You need to learn the language and write the code yourself. Code written by any random individual may contain features that you're not yet familiar with or errors that would require some advanced knowledge of the language to correct. In particular, simplezz's code is not valid C#.

 

You should be able to tell exactly why the result your code gives is incorrect, if you actually wrote it. It does exactly what you told it to do. You can observe what it does line by line by debugging it (press F10, F11 to step inside functions, in Visual Studio). By the way, I can't compile the code you posted because it's missing the definition of the type "Roman". Could you post it? Actually, did you write the class "Roman"? If you didn't, scrap that code. Scrap any code you didn't write. Again, you'll go nowhere by copying other people's code. It's a waste of your time and everyone else's time to come here and ask for help with code you copy-pasted from somewhere.

 

Can you at least identify what it is about your code that you don't understand?

[Joe User]

I'm not a C# developer, but I can see what looks like a logic error.

 

Take the time to step through your code, you'll find it.

 

If not, ask your teacher for help.

[Andre S. Veteran]

Well there are some very obvious ones:

if (rom[i] < rom[i])

This is always false because you're comparing a value with the very same value. A value cannot be smaller than itself.
Similarly:

else if(rom[i] >= rom[i])

This is always true, for the same reason as above.
 

Console.WriteLine("dc");

This prints the string "dc" instead of the variable dc.
 
Also there are some structural issues:
 

try
{
    for ( i = 0; i < 10; i++)
    {
        switch (rm.RomanNumber[i])
        {
            // (...)
        }
    }
}
catch (IndexOutOfRangeException) {}

This is a terrible way to iterate over the string - it will fail for any string longer than 10 -, much better would be to remove the try/catch and compare i with rm.RomanNumber.Length instead of 10.

Finally whatever the definition of class "Roman" is, it serves no useful purpose here. Simply don't assign the input string to rm.RomanNumber and use it directly, and you can remove the Roman variable and the Roman class entirely.

[Seahorsepip Veteran]

Just write your code step by step, here's a example written in javascript with explanation, you can do rewrite the same code step by step in C. Make sure to test your code every step.

 

Step 1:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

Define a array with arrays as variable, I suggest to look at some info about arrays in C.

 

 

Step 2:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
}

Create a function with 2 variables, first a variable dec that contains the starting decimal value 0. Then a variable rom that contains the roman input.

 

 

Step 3:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
    }
}

Create a for loop for each roman number in the array. I suggest to look about some info about for loops in C.

 

 

Step 4:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
        }
    }
}

Check if the first letter(s) of the roman input are equal with the current roman number of the for loop iteration.

 

 

Step 5:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length);
        }
    }
}

When the first letter(s) of the roman input are equal with the current roman number then remove those first letter(s) from the rom variable.

 

Step 6:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length);
            dec += roman[x][0];
        }
    }
}

When the first letter(s) of the roman input are equal with the current roman number then sum up the  dec variable with the current roman number.

 

Step 7:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length); //Removes first letter(s) when equal
            dec += roman[x][0];
            x = -1;
        }
    }
}

When the first letter(s) of the roman input are equal with the current roman number then start the whole loop over again to check the first letter(s) of the modified rom variable.

 

Step 8:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length); //Removes first letter(s) when equal
            dec += roman[x][0];
            x = -1;
        }
    }
    alert(dec);
}

The variable dec contains the decimal value of the roman input after the for loop, in this example we show it in a alert after the for loop.

 

 

This wasn't that hard was it? Just make sure that you understand arrays and for loops in C.

[+theblazingangel MVC]

@OP. Understand that your code is walking through the string interpreting each character one at a time entirely independently of any other characters around it. As you can see, this means that you get some incorrect results.
 
Recognising that the order of characters can affect their individual meaning from addition to subtraction, it is clear that your current algorithm is insufficient to correctly interpret strings. Put the code to one side. First go and make sure you understand the exact correct rules for correctly formatted roman numerals. Second, think logically about how to approach parsing a string of them to correctly interpret value. It could require perhaps a loop to go over each character, with a nested loop with which for each character you look ahead at all the following characters to determine if there is a larger valued characters following it. Or instead of just searching for single digits, you could expand to searching for matching multi-digit pieces as demonstrated above. Perhaps it could help if you examined the string from right to left (backwards) instead of left to right. (I'm not certain which of these may actually be helpful, I haven't put much thought towards a solution to this myself, I'm mearly throwing suggestions at you of things to consider that you might not have thought of).
 

  On 12/09/2014 at 17:02, Seahorsepip said:

Just write your code step by step, here's a example written in javascript with explanation, you can do rewrite the same code step by step in C. Make sure to test your code every step.

<snip>
 
Step 7:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length); //Removes first letter(s) when equal
            dec += roman[x][0];
            z = -1;
        }
    }
}

When the first letter(s) of the roman input are equal with the current roman number then start the whole loop over again to check the first letter(s) of the modified rom variable.
 
Step 8:

var roman = [[1000,"M"],[900,"CM"],[500,"D"],[400,"CD"],[100,"C"],[90,"XC"],[50,"L"],[40,"XL"],[10,"X"],[9,"IX"],[5,"V"],[4,"IV"],[1,"I"]];

function todec() {
    var dec = 0;
    var rom = "Roman input";
    for(x=0;x<roman.length;x++) {
        if(rom.substring(0, roman[x][1].length) == roman[x][1]) {
            rom = rom.substring(roman[x][1].length); //Removes first letter(s) when equal
            dec += roman[x][0];
            z = -1;
        }
    }
    alert(dec);
}

The variable dec contains the decimal value of the roman input after the for loop, in this example we show it in a alert after the for loop.
 
 
This wasn't that hard was it? Just make sure that you understand arrays and for loops in C.

 

 
You lost me at z = -1, did you mean x = -1?? :p
 
I'm going to be cheeky and take the opportunity to throw a little optimisation tip at you, just in case you don't yet know it:
Instead of
 

for (x=0; x<y; x++) {

write
 

for (x=0; x<y; ++x) {

You may already know that pre-increment/decrement is faster than post, because post creates a copy of the original value before incrementing, whereas pre doesn't, and so you should only do post when you actually need to. With the for loop here it may be a little confusing to some about which is needed; this increment operation actually takes place at the end of each loop not the start, so post incrementation is entirely unnecessary. Your compiler may very well optimise this away for you automatically, but personally I think it's better to always explicitly use pre unless you really specially need post.

 

Moderator note: the rest of the pre-increment/post-increment conversation was split to its own thread here.

[simplezz]

  On 12/09/2014 at 03:43, Andre S. said:

In particular, simplezz's code is not valid C#.

I did add a caveat that I wrote it without compilation or testing. It should give a general idea of how to achieve the desired result though. I neither write C# nor develop on Windows, so I can't really test it.

  On 12/09/2014 at 03:43, Andre S. said:

Can you at least identify what it is about your code that you don't understand?

To be fair Andre, I think you're being a bit harsh with him. In particular, closing the other thread before he could even reply was a little heavy handed IMO.

To the OP, I think the errors are probably caused by this part:

		struct NumeralPair {
			public int 	value;
			public string	roman;
		};
	
		NumeralPair[] PairTable = new NumeralPair[] {
			{ M, "M" }, { M-C, "CM"}, { D, "D"}, { D-C, "CD" }, { C, "C" }, { C-X, "XC" }, 
			{ L, "L" }, { L-X, "XL" }, { X, "X" }, { X-I, "IX" }, { "V" }, { V-I, "IV" }, { I, "I" }
		};

I'm not sure of the correct C# version of that because I don't really program in that language. Perhaps someone more experienced in it can fix it.

I'd implement it in C using a struct and array:

		typedef struct {
			int 	value;
			char	roman[2];
		} NumeralPair;
	
		NumeralPair PairTable[] = {
			{ M, "M" }, { M-C, "CM"}, { D, "D"}, { D-C, "CD" }, { C, "C" }, { C-X, "XC" }, 
			{ L, "L" }, { L-X, "XL" }, { X, "X" }, { X-I, "IX" }, { "V" }, { V-I, "IV" }, { I, "I" }
		};

Maybe a C# expert can convert it to valid C#.

[simplezz]

Okay, I went ahead and installed mono (Ugh!). I compiled it and did a basic input test and it worked.

using System;
using System.Diagnostics;

class RomanApp {
	static void Main ( string[] args ) 
	{
		Console.WriteLine ( "Please enter a roman numeral" );
		
		Roman numeral = new Roman ( Console.ReadLine() );
		Console.WriteLine ( "You entered the roman numeral {0} which equals {1} in the decimal system.", numeral.ToNum(), numeral.ToDec() );
    
		Console.WriteLine ( "Please enter a decimal number (integer)" );
		
        	Roman numeral2 = new Roman ( Convert.ToInt32 ( Console.ReadLine() ) );
		Console.WriteLine ( "You entered the decimal {0} which equals {1} in the roman numeral system.", numeral2.ToDec(), numeral2.ToNum() );
		
		numeral += numeral2;
		Console.WriteLine ( "The two numerals added equal: {0}", numeral.ToNum() );
    }
}

class Roman {
	
	private const int M = 1000;	/* milium / mille */
	private const int D = 500;	/* quingenti */
	private const int C = 100;	/* centum */
	private const int L = 50;	/* quinquaginta */
	private const int X = 10;	/* decem */
	private const int V = 5;	/* quinque */
	private const int I = 1;	/* unus */

    	struct NumeralPair {
		public int 	value;
		public string	roman;
        	public NumeralPair ( int value, string roman ) {
            		this.value = value;
            		this.roman = roman;
        	}            
	};
	
	private NumeralPair[] PairTable = {
        	new NumeralPair ( M, "M" ), new NumeralPair ( M-C, "CM" ), new NumeralPair ( D, "D" ), new NumeralPair ( D-C, "CD" ), 
        	new NumeralPair ( C, "C" ), new NumeralPair ( C-X, "XC" ), new NumeralPair ( L, "L" ), new NumeralPair ( L-X, "XL" ), 
        	new NumeralPair ( X, "X" ), new NumeralPair ( X-I, "IX" ), new NumeralPair ( V, "V" ), new NumeralPair ( V-I, "IV" ), 
        	new NumeralPair ( I, "I" )
	};
	
	private string num; 		/* roman numeral */
	private int    dec; 		/* decimal equivalent */
	
	public Roman ( ) {
	
	}
	
	public Roman ( string numeral ) {
		StoreNum ( numeral );
	}
	
	public Roman ( int dec ) {
		StoreDec ( dec );
	}

	private void StoreNum ( string numeral ) {
		this.num = numeral;
		this.dec = NumeralToDec ( numeral );
	}

    	private void StoreDec ( int dec ) {
		this.dec = dec;
		this.num = DecToNumeral ( dec );
	}
        
    	private int NumeralToDec ( string numeral ) {
		int count = 0;
		int dec;
		
		for ( int i = 0; i < numeral.Length; i++ ) {

			dec = CharToDec ( numeral [ i ] );
			if ( i < numeral.Length -1 && dec < CharToDec ( numeral [ i + 1 ] ) ) {
				Debug.Assert ( dec == I || dec == X || dec == C );
				count -= dec;
			} else {
                		count += dec;
            		}
		}

	        return count;    
	}
	
	private string DecToNumeral ( int dec ) {

		string numeral = "";

		foreach ( NumeralPair pair in PairTable ) {
		
			while ( dec >= pair.value ) {
				numeral += pair.roman;
				dec 	-= pair.value;
			}
		}
		
		return numeral;
	}
	
	private int CharToDec ( char numeral ) {
		int dec = 0;
		
		switch ( numeral ) {
		case 'M':
		case 'm':
			dec = M;
			break;
		case 'D':
		case 'd':
			dec = D;
			break;
		case 'C':
		case 'c':
			dec = C;
			break;
		case 'L':
		case 'l':
			dec = L;
			break;
		case 'X':
		case 'x':
			dec = X;
			break;
		case 'V':
		case 'v':
			dec = V;
			break;
		case 'I':
		case 'i':
			dec = I;
			break;
		default:
			/* todo: unrecognised roman numeral - raise exception */
			Debug.Assert ( false );
            		break;
		}

		return dec;
	}
	
	public static Roman operator +( Roman left, Roman right ) {
        	return new Roman ( left.dec + right.dec ); 
	}

	public int	ToDec ( ) { return dec; }
	public string	ToNum ( ) { return num; }
}

$ mcs roman.cs 
$ mono roman.exe
Please enter a roman numeral
MMXIX
You entered the roman numeral MMXIX which equals 2019 in the decimal system.
Please enter a decimal number (integer)
2019
You entered the decimal 2019 which equals MMXIX in the roman numeral system.
The two numerals added equal: MMMMXXXVIII

I have to say, I really hate the way C# array initialisers work. It's so much nicer in C.

[Andre S. Veteran]

  On 14/09/2014 at 14:15, simplezz said:

I have to say, I really hate the way C# array initialisers work. It's so much nicer in C.

The array initialization syntax is the same actually, it's the struct initialization syntax that's different; there's no general shortcut with braces like you have in C (note that struct in C# means very specifically user-defined value type and is not often used). But there are others and better ways of doing what you are doing with structs and arrays in C#. In C# you'd use an enum rather than declaring constants, and you could simply call ToString() on it to get the equivalent string representations. So you could get strongly typed roman numerals with their string representations and integral values this way:

enum RomanDigit {
    I = 1, II = 2, III = 3,
    IV = 4, V = 5,
    // etc.
}

var stringRepresentation = RomanDigit.I.ToString("g");
var numericValue = (int)RomanDigit.I;

In general if you want to do some mapping between a set of values and another set of values, you'd use a Dictionary<Key, Value> instead of an array of some custom pair type.

var dict = new Dictionary<int, string> { {1, "I"}, {2, "II"} };

[simplezz]

  On 15/09/2014 at 01:56, Andre S. said:

In general if you want to do some mapping between a set of values and another set of values, you'd use a Dictionary<Key, Value> instead of an array of some custom pair type.

var dict = new Dictionary<int, string> { {1, "I"}, {2, "II"} };

You're right. I tested that out and a Dictionary is much nicer for initialisation. It also handily eliminates the need for a switch statement.

 

using System;
using System.Collections.Generic;

class RomanApp {
	static void Main ( string[] args ) 
	{
		Console.WriteLine ( "Please enter a roman numeral" );
		
		Roman numeral = new Roman ( Console.ReadLine() );
		Console.WriteLine ( "You entered the roman numeral {0} which equals {1} in the decimal system.", numeral.ToNum(), numeral.ToDec() );
    
		Console.WriteLine ( "Please enter a decimal number (integer)" );
		
        	Roman numeral2 = new Roman ( Convert.ToInt32 ( Console.ReadLine() ) );
		Console.WriteLine ( "You entered the decimal {0} which equals {1} in the roman numeral system.", numeral2.ToDec(), numeral2.ToNum() );
		
		numeral += numeral2;
		Console.WriteLine ( "The two numerals added equal: {0}", numeral.ToNum() );
    }
}

class Roman {
	
	private Dictionary < string, int > PairTable = new Dictionary < string, int > ( StringComparer.OrdinalIgnoreCase ) 
		{ { "M", 1000 }, { "CM", 900 }, { "D", 500 }, { "CD", 400 }, { "C", 100 }, { "XC", 90 }, { "L", 50 }, 
		{ "XL", 40 }, { "X", 10 }, { "IX", 9 }, { "V", 5 }, { "IV", 4 }, { "I", 1 } };
    	
	private string num;		/* roman numeral */
	private int    dec;		/* decimal equivalent */
	
	public Roman ( string numeral ) {
		StoreNum ( numeral );
	}
	
	public Roman ( int dec ) {
		StoreDec ( dec );
	}

	private void StoreNum ( string numeral ) {
		this.num = numeral;
		this.dec = NumeralToDec ( numeral );
	}

    	private void StoreDec ( int dec ) {
		this.dec = dec;
		this.num = DecToNumeral ( dec );
	}
        
    	private int NumeralToDec ( string numeral ) {
		int count = 0;
		int dec;
		
		for ( int i = 0; i < numeral.Length; i++ ) {

			dec = CharToDec ( numeral [ i ] );
			if ( i < numeral.Length -1 && dec < CharToDec ( numeral [ i + 1 ] ) ) {
				/* Debug.Assert ( dec == I || dec == X || dec == C ); */
				count -= dec;
			} else {
                		count += dec;
            	        }
		}

		return count;    
	}
	
	private string DecToNumeral ( int dec ) {
		string numeral = "";
		
		foreach ( KeyValuePair < string, int > pair in PairTable ) {
		
			while ( dec >= pair.Value ) {
				numeral += pair.Key;
				dec 	-= pair.Value;
			}
		}
		
		return numeral;
	}
	
	private int CharToDec ( char numeral ) {
		return PairTable [ Convert.ToString ( numeral ) ];
	}
	
	public static Roman operator +( Roman left, Roman right ) {
        	return new Roman ( left.dec + right.dec ); 
	}

	public int	ToDec ( ) { return dec; }
	public string	ToNum ( ) { return num; }
}