Char <-> Char[i]

[Omnia Vanitas]

I've been doing exercises\tutorials to learn\practice. I thought I would make a converter to\from Roman Numerals because I'd done it w/ other languages & figured it (w)could employ a variety of basic principles.

I decided to perform the conversion in phases since I'm still too clumsy to negotiate with advanced string manipulation and all the error correcting\anticipating that might entail. I'm sure there are better ways to do it, but that's not the point.

For some reason when I compile the code, 'Phase3' does not work the way it should. If I use "=" it sets the variable 'Number' to 1000000 and claims the Numeral entered was a black smiley. I assumed this was related to pointers, and mistaking comparison for assignment, but 'Number' should still only be 1000000 if 'Numeral' was 'A' or 'a', otherwise it should be 0.

If I use "==" it doesn't have that problem, but when I execute the code, it sets 'Number' to 1. 'Number' should never be '1' unless 'Numeral[i]' is 'I' or 'i'. The only other problems I can think of is if I made the array too large or if I'm over shooting the array boundary with the iterator, but that really doesn't explain why my code returns false positives and ignores genuine matches:

--- Start ---

include <cstdlib>

include <iostream>

using namespace std;

int Number[255];
char Numeral[255];
int Result = 0;
int Length = 255;
int Extended = 0;
int i = 0;
int Converting = 0;

void Phase1 ()
{
for (i=0; i<= Length; i++)
{
Number[i] = 0;
Numeral[i] = 0;
}
Converting = 2;
return;
}

void Phase2 ()
{
cout << "Please enter a Numeral: ";
cin >> Numeral;
Converting = 3;
return;
}

void Phase3 ()
{
for (i=0; i<=Length; i++)
{
if (Numeral[i] == 'I' || 'i') { Number[i] = 1; Extended = 0; }
else if (Numeral[i] == 'V' || 'v') { Number[i] = 5; Extended = 0; }
else if (Numeral[i] == 'X' || 'x') { Number[i] = 10; Extended = 0; }
else if (Numeral[i] == 'L' || 'l') { Number[i] = 50; Extended = 0; }
else if (Numeral[i] == 'C' || 'c') { Number[i] = 100; Extended = 0; }
else if (Numeral[i] == 'D' || 'd') { Number[i] = 500; Extended = 0; }
else if (Numeral[i] == 'M' || 'm') { Number[i] = 1000; Extended = 0; }
else if (Numeral[i] == 'Y' || 'y') { Number[i] = 5000; Extended = 1; }
else if (Numeral[i] == 'Z' || 'z') { Number[i] = 10000; Extended = 1; }
else if (Numeral[i] == 'F' || 'f') { Number[i] = 50000; Extended = 1; }
else if (Numeral[i] == 'H' || 'h') { Number[i] = 100000; Extended = 1; }
else if (Numeral[i] == 'B' || 'b') { Number[i] = 500000; Extended = 1; }
else if (Numeral[i] == 'A' || 'a') { Number[i] = 1000000; Extended = 1; }
else { Number[i] = 0; }
// cout << "Conversion Phase 3 Iteration: " << i << " Numeral: " << Numeral[i] << " Number: " << Number[i] << "\n";
// system("PAUSE");
// ^ Dump Variables & Pause @ Iteration
}
Converting = 4;
return;
}

void Phase4 ()
{
for (i=0; i<=Length; i++)
{
if (Number[i] >= Number[i+1]) Result = (Result + Number[i];
else if (Number[i] < Number[i+1]) Result = (Result - Number[i];
}
Converting = 5;
cout << Numeral << " = " << Result << "\n";
return;
}

int main(int argc, char *argv[])
{
Phase1();
if (Converting == 2) Phase2();
if (Converting == 3) Phase3();
if (Converting == 4) Phase4();
system("PAUSE");
return EXIT_SUCCESS;
}
--- End ---

Any ideas what I'm doing wrong that would create this phenomena?

[Richard Kopcke]

For starters: in the loops addressing Number and Numeral i<length

[Omnia Vanitas]

Alright, that makes sense. I thought it was resolving to 1 because it compared 'I' first. Was trying to figure out if it was somehow related to ASCII codes or what. Wouldn't have guessed that it was comparing my comparisons with themselves for lack of a second variable.

That fixed it though. I'll fix the iterator bug and work on optimizing the conversion now that I have a working design. I'll remember toupper & cctype when I eliminate the phases. =)

Thanks for the quick & thorough responses guys.

[cpns]

This condition:

if (Numeral[i] == 'A' || 'a')

and all like it do not mean what you think they do. First the sub-expression ('A' || 'a') is evaluated, which is a boolean expression that will always be true. Casting true to an integer, yields 1, and character 1 resolves in your case to the glyph you see.

The condition should be:

if( Numeral[i] == 'A' || Numeral[i] == 'a' )

or perhaps more succinctly:

if( toupper( Numeral[i] ) == 'A' )

where toupper() is declared in <cctype>.

Also what Richard Kopcke said; you have an out-by-one error on your loop termination, Numeral[Length] references the character past the end of the array.

I recommend that you add -Wall and -Werror to your compiler settings.

Clifford