Problem with roman numeral converter

[manderson1]

Hello. Im trying to make a program that converts roman numerals to arabic numerals. I keep getting a error saying 'unresolved external', and I do not know why this is.

This is my code:

  #include<iostream>

using namespace std;

class romanType
{
private:
	string romanNum;
	int decimalNum;
public:
	void setRoman(string romanString);
	void romanToDecimal();
	void printDecimal();
	void printRoman();
};

int main()
{
	romanType roman();
	string romanString;

	cout << "Please enter roman number: ";
	cin >> romanString;
	
	setRoman(romanString);
	
}

void romanType::setRoman(string romanString)
{
	int previous = 1000;
	int sum = 0;

	for(int i = 0; i<romanString.length; i++)
	{
		cout << romanString[i] << " ";
		if(romanString[i] == 'M')
		{
			sum = sum + 1000;
			if(previous<1000)
			{
				sum = sum - (2 * previous)
			}
			previous = 1000;
		}

		else if(romanString[i] == 'D')
		{
			sum = sum + 500;
			if(previous<500)
			{
				sum = sum - (2 * previous)
			}
			previous = 500;
		}

		else if(romanString[i] == 'C')
		{
			sum = sum + 100;
			if(previous<100)
			{
				sum = sum - (2 * previous)
			}
				previous = 100;
		}

			else if(romanString[i] == 'L')
		{
			sum = sum + 50;
			if(previous<50)
			{
				sum = sum - (2 * previous)
			}
				previous = 50;
		}

		else if(romanString[i] == 'X')
		{
			sum = sum + 10;
			if(previous<10)
			{
				sum = sum - (2 * previous)
			}
				previous = 10;
		}

			else if(romanString[i] == 'V')
		{
			sum = sum + 5;
			if(previous<5)
			{
				sum = sum - (2 * previous)
			}
				previous = 5;
		}

		else if(romanString[i] == 'I')
		{
			sum = sum + 1;
			previous = 1;
		}
	}

}

[andywestken]

When the linker complains about the unresolved external, it should be naming it. So, what function is it referring to?

It's complaining that while you're told it about function (declared it), and used it, that you haven't provided the actual function (defined it).

Andy

[giblit]

I'm guessing because you never made a constructor in your class. Also what are you trying to do on line 19 call a function or name an object of your class?
To name an object try this instead
romanType roman; then on line 25 it would be

roman.setRoman( romanString );

[manderson1]

Okay, so I would call the function by having line 25 as roman.setRoman(romanString);, and having line 29 as void romanType::setRoman(string romanString)?

@giblit: yes, I was naming an object.
@andy: I am referring to the void romanType::setRoman(string romanString) function.

[giblit]

manderson1 wrote:

Okay, so I would call the function by having line 25 as roman.setRoman(romanString);, and having line 29 as void romanType::setRoman(string romanString)?

Yeah that is how you would do it.
By the way have you considered using a switch instead of all the if else?

siwtch( romanString[i] )
{
case 'M':
    sum += 1000; //you += means the old value + something or sum = sum + 1000
    if( previous < 1000 )
    {
    sum -= ( 2 * previous ); //same as before but - instead of +
    }
    previous = 1000;
    break;
case 'D':
//stuff
break;
/* ^^ do that for all of them instead of all the else if's ^^*/
}

[manderson1]

Okay, thank you. Also, Is there something else that I am doing wrong?, because I am still getting the 'unresolved external' error.

[giblit]

What exactly does it try and I suppose I can try and compile it myself lol
Oh duh lol you are declaring the class functions after the main. move the main function under the class function declarations.

//class
//class function declaration
//int main 

[Daleth]

The declarations are already within the class definitions. What Gilbit calls the declarations are the function definitions.

To confirm, you have done as Gilbit first suggested?

romanType roman; //omitted ()
//...
roman.setRoman(romanstring);

[giblit]

oh yeah sorry the definitions of the functions
I'm pretty sure what the op is trying to do is something like this

int main( int argc , char **argv)
{
    function();
}

void function( void )
{
    std::cout << "I am not going to be called ever!" << std::endl;
}

unless classes act as a prototype I normally just put it before the main.

Actually forget that...the class acts as the prototype I dont know what I was thinking.

I think the error is just because you didn't name the object correctly earlier.

Also on line 34 .length is a function so you need to put .length();

Line 42 missing a semicolon
line 52 missing a semicolon
line 63 missing a semicolon
line 72 missing a semicolon
line 82 missing a semicolon
line 92 missing a semicolon

[manderson1]

@Daleth: Yes, I did.
@giblit: Yes i have thought about using 'switch', but I'd rather do it as I am doing it now to avoid more confusion. And it doesn't try anything, I just get the 'unresolved external' error.

[giblit]

It compiled fine for me after fixing the issue with the object.
Maybe try clean building? Because I didn't have that unresolved external error

[manderson1]

@gilbit:
When you compiled it, was it exactly like this? And what is 'clean building'?

#include<iostream>

using namespace std;

class romanType
{
private:
	string romanNum;
	int decimalNum;
public:
	void setRoman(string romanString);
	void romanToDecimal();
	void printDecimal();
	void printRoman();
};

romanType roman;

int main()
{
	
	string romanString;

	cout << "Please enter roman number: ";
	cin >> romanString;
	
	roman.setRoman(string romanString);
	
}

void romanType::setRoman(string romanString)
{
	int previous = 1000;
	int sum = 0;

	for(int i = 0; i<romanString.length(; i++)
	{
		cout << romanString[i] << " ";
		if(romanString[i] == 'M')
		{
			sum = sum + 1000;
			if(previous<1000);
			{
				sum = sum - (2 * previous)
			}
			previous = 1000;
		}

		else if(romanString[i] == 'D')
		{
			sum = sum + 500;
			if(previous<500);
			{
				sum = sum - (2 * previous)
			}
			previous = 500;
		}

		else if(romanString[i] == 'C')
		{
			sum = sum + 100;
			if(previous<100);
			{
				sum = sum - (2 * previous)
			}
				previous = 100;
		}

			else if(romanString[i] == 'L')
		{
			sum = sum + 50;
			if(previous<50);
			{
				sum = sum - (2 * previous)
			}
				previous = 50;
		}

		else if(romanString[i] == 'X')
		{
			sum = sum + 10;
			if(previous<10);
			{
				sum = sum - (2 * previous)
			}
				previous = 10;
		}

			else if(romanString[i] == 'V')
		{
			sum = sum + 5;
			if(previous<5);
			{
				sum = sum - (2 * previous)
			}
				previous = 5;
		}

		else if(romanString[i] == 'I')
		{
			sum = sum + 1;
			previous = 1;
		}
	}

}

[TheIdeasMan]

@manderson1

IMO, you shouldn't have the setRoman function. Instead provide a constructor that takes a string & uses an initiliser list to set the romanNum variable.

You have a romanToDecimal function but it isn't defined or used anywhere.

Your setRoman function is of type void & doesn't set any member variables, so the local sum variable can't be used by anything. So that code should be in the romanToDecimal function and sum should be a member variable.

As giblit said use a switch, but also make sure it has a default clause to catch any bad input.

Also avoid having global variables like you do on line 17 - at least put it in main() instead.

This code sum = sum + 5; can be better written as sum += 5; if the value is 1 then sum++;

Try to avoid having 'magic numbers' in your code - numbers like 1000, 500 etc should be declared & initialised as const int at the beginning of main(). Then you can use that variable throughout your code.

HTH

[giblit]

You didn't do any of the things I said so no it looks different than what I did

[Daleth]

I'm just gonna throw in here that on line 27, you are declaring a function with no return type, not calling a function.

[TheIdeasMan]

@Daleth

That was the case with the OP's first lot of code on line 25, but line 27 in the second version calls the setroman function.

[Daleth]

Line 27:

@gilbit:
When you compiled it, was it exactly like this? And what is 'clean building'?
...

int main()
{
	
	string romanString;

	cout << "Please enter roman number: ";
	cin >> romanString;
	
	roman.setRoman(string romanString);
	
}

This will be seen by the compiler as an attempt to declare a function?

[giblit]

Think he means because op is declaring a new string on call he has string romanString instead of just romanString

[TheIdeasMan]

There is a global object on line 17 (roman), and setRoman is one of it's functions.

[manderson1]

@Daleth: Yes, wouldn't it?
@TheIdeasMan: Right, isn't that correct?