C++

Forum

 
Having trouble with arrays output !!! Please Help!!!

jothy (3)
Hello, I'm a complete newbie in programming and this is the first programming class I'm taking.
We are supposed to make a program that convert a double into a Uppercase Roman Number and then convert the Uppercase format into a lowercase.And then display the first 20 roman numbers using arrays where the initial number is incremented by 1.
The output is supposed to be something like this:
initial Upcase Lowercase
125 CXXV cxxv
126 CXXVI cxxvi
127 CXXVII cxxvii
(and so on...)

The problem is that my program displays a pretty messed up output. Data are not displayed at the position I set for them.
This is the output I get :
cxxv 125 CXXV CXXVCXXVI

cxxvi 126 CXXVI CXXVICXXVII

cxxvii 127 CXXVII CXXVIICXXVIII


Can somebody help me please??


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
 
  #include "stdafx.h"
#include <iostream>
#include <string>
#include<cctype>
#include<iomanip>
#include<cmath>


using namespace std;
string  roman(double num);
string MyString(string stringtest);
string MyString(string string1);
void lowercase(string stringtest);

int main()
{
	double number = 0.0;
	string StringArray[30][2];
		double DoubleArray[30][1];

	while (true) {

		cout << "Enter a number:";
		cin >> number;
		if (number < 1 || number > 5000) {
			cout << "your input is not valid, please try again" << endl;
		}
		else {
			break;
		}
	}
	

	for (int iRow = 0; iRow < 20; iRow++) {
		StringArray[iRow][0] = roman(number);
		StringArray[iRow][1] = MyString(StringArray[iRow][0]);
		DoubleArray[iRow][0] = number;
		number++;
		
		for (int iCol = 0; iCol < 1; iCol++) {

			cout << setw(10) << DoubleArray[iRow][iCol];
		}
			
		for (int iCol = 0; iCol < 2; iCol++) {

        cout << setw(10) << StringArray[iRow][iCol];
		}
		
		
	}
	cout << endl;
	system("pause");
	return 0;
}

string  roman(double num) {

	int   intnum, m, d, c, l, x, v, i, n;
	intnum = (int)num;
	string string1 = "";

	m = intnum / 1000;
	d = ((intnum % 1000) / 500);
	c = ((intnum % 500) / 100);
	l = ((intnum % 100) / 50);
	x = ((intnum % 50) / 10);
	v = ((intnum % 10) / 5);
	i = (intnum % 5);
	n = m + d + c + l + x + v + i;


	while (n > 0)
	{

		{
			for (m; m>0; m--)
				string1.append("M");
		}
		{
			for (d; d>0; d--)
				string1.append( "D");
		}
		{
			for (c; c>0; c--)
				string1.append("C");
		}
		{
			for (l; l>0; l--)
				string1.append("L");
		}
		{
			for (x; x>0; x--)
				string1.append("X");
		}
		{
			for (v; v>0; v--)
				string1.append("V");
		}
		{
			for (i; i>0; i--)
			string1.append("I");
		}
		n--;


	}
	cout << string1 << endl;
	cout << endl;
	return string1;
}

string MyString(string string1) {
	/*string lowerRoman = "";
	for (int i = 0; i < string1.length(); i++) {
		char c = string1.at(i);
		
		char d = tolower(c);
		lowerRoman.append(i, d);
	}
	return lowerRoman;*/
	
	lowercase(string1);
	return string1;
}

void lowercase(string stringtest) {
	for (unsigned int i = 0; i < stringtest.length(); ++i) {
		stringtest[i] = tolower(stringtest[i]);
		cout << stringtest[i];
	}
}


lastchance (6980)
@jothy

You are trying to do far too many things at once. Forum users will baulk at trying to sort out too much confusion. Break it down into smaller tasks and check that each one works individually before trying to throw the lot together.

General comments:
- I don't think the Romans did fractions, so you will be converting an int, not a double;
- you do not need arrays to do any of this: just write out your answer as a table (not the same as an array).

Task 1 - convert an int (not a double) into a roman numeral. Correct to have a string-returning function, but your conversion is both wrong and very over-complicated. Remember that roman numerals use "subtraction notation" in places - e.g. 4 is IV, not IIII. Get this working first, and check it separately, before doing anything else.

Task 2 - convert a string to lowercase. Given what you intend to do this probably has the prototype
string lowercase( string stringtest );
As it stands, your function argument is passed by value, not reference, so it won't change anything in the calling program. Check this routine separately before going on.

Task 3 - write out a table. Just loop on i and cout, on each line,
i, roman(i), lowercase(roman(i))
(which you could make slightly more efficient by evaluating roman(i) in the loop, rather than twice).

In short, ask for help with small specific parts. Also don't include unnecessary or commented-out code (like your Mystring routine).
jothy (3)
@lastchance
Thank you very much for your feedback.
You are making very good points but the problem is that it's an assignment and I have to follow a set of rules.

Task 1 - My teacher asked for the long-form notation, he is therefore expecting 4 to be IIII. But you are right about the int conversion, I fixed it.

Task 2 - I actually checked the function separately and it was working just fine (separately).
Task 3 - I am required to do it with 2 parallel arrays, otherwise I get no credit.

My problem is why is the lowercase appearing first in the row while I set it to appear last (The order is: Integer Upcase Lowercase)?
And why am I getting that extra Uppercase Roman number at the end of the row since I am supposed to have 3 rows only? And how can I fix that?
Any help would be very appreciated.
Last edited on
lastchance (6980)
Well, I've tried to make your code work with minimal interference - you can check it on cpp-shell (the little gear-wheel to the top right of code samples once you have logged in). Also note any warnings that throws up - they aren't errors, but they indicate where your code could be improved.

A lot of your spurious output comes from the cout lines that you left in roman() - presumably whilst developing/debugging. You need to "walk through" your code with pen and paper, following the calculations and output it undertakes.

You can see from my comments that I think there is quite a lot that you might do to make the code more efficient.

I'm sorry, but I think arrays are completely unnecessary here ... but I've left StringArray[][] in for now. It really serves no purpose.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
 
// #include "stdafx.h"    // put back for Visual Studio if used
#include <iostream>
#include <string>
#include<cctype>
#include<iomanip>
#include<cmath>
using namespace std;

// Function Prototypes
string roman( int num );                  // <==== int, not double
                                          // <==== removed Mystring
string lowercase(string stringtest);      // <==== return a string



int main()
{
        const int IMIN = 1, IMAX = 20;    // <==== define range
        int number = 0;                   // <==== int, not double
        string StringArray[IMAX+1][2];    // <==== don't use magic numbers
                                          // <==== DoubleArray not needed
                                          // <==== next section removed as irrelevant
        

        for (int iRow = IMIN; iRow <= IMAX; iRow++)          // <==== don't use "magic numbers"
        {
                StringArray[iRow][0] = roman( iRow );        // <==== just use the loop index!!
                StringArray[iRow][1] = lowercase( roman( iRow ) );
                                          // <==== lines removed as irrelevant
                
                cout << setw(4 ) << iRow << " "
                     << setw(10) << StringArray[iRow][0] << " " 
                     << setw(10) << StringArray[iRow][1] << endl;
        }

//      system("pause");       // put back for Visual Studio if used
}

string roman( int num )                   // <==== int, not double
{
        int m, d, c, l, x, v, i, n;
        string string1 = "";

        m = num / 1000;                   
        d = ((num % 1000) / 500);         // <==== this lot is really painful! consider rewriting it
        c = ((num % 500) / 100);          // <====
        l = ((num % 100) / 50);           // <====
        x = ((num % 50) / 10);            // <====
        v = ((num % 10) / 5);             // <====
        i = (num % 5);                    // <==== 
        n = m + d + c + l + x + v + i;

        while (n > 0)                                   // <==== there are much simpler ways of doing this
        {
                for (m; m>0; m--) string1.append("M");
                for (d; d>0; d--) string1.append("D");
                for (c; c>0; c--) string1.append("C");
                for (l; l>0; l--) string1.append("L");
                for (x; x>0; x--) string1.append("X");
                for (v; v>0; v--) string1.append("V");
                for (i; i>0; i--) string1.append("I");
                n--;


        }
        // <==== removed cout statements (presumably left over from development)
        return string1;
}

// <==== removed MyString

string lowercase( string stringtest )      // <===== note that stringtest is being passed by VALUE
{
        for (unsigned int i = 0; i < stringtest.length(); ++i)
        {
                stringtest[i] = tolower(stringtest[i]);
        }
        return stringtest;                 // <===== this is the return value of the function, NOT a modified argument
}



FWIW, and since your teacher won't let you use "real" roman-numeral notation, here is my conversion routine:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 
#include <iostream>
#include <string>
using namespace std;

//======================================================================

string dec( int i, char sym, char sym5, char sym10 )       // return roman 'digit' as a string
{
   string s = "";
   if      ( i == 9 ) s = ( s + sym ) + sym10;
   else if ( i == 4 ) s = ( s + sym ) + sym5;
   else if ( i >= 5 ) s += sym5 + string( i - 5, sym );
   else if ( i >= 0 ) s += string( i, sym );
   return s;
}

//======================================================================

string roman( int n )                  // convert integer num to roman numeral (as a string)
{
   int m = n / 1000;   n %= 1000;
   int c = n / 100 ;   n %= 100 ;
   int x = n / 10  ;   n %= 10  ;
   int i = n;

   return string( m, 'M' ) 
           + dec( c, 'C', 'D', 'M' )
           + dec( x, 'X', 'L', 'C' )
           + dec( i, 'I', 'V', 'X' );
}

//======================================================================

int main()
{
   int n = 1;
   while ( n > 0 )
   {
      cout << "Input n (or 0 to end): ";
      cin >> n;
      if ( n > 0 ) cout << "Roman numeral: " << roman( n ) << "\n\n";
   }
}

//====================================================================== 

Input n (or 0 to end): 34
Roman numeral: XXXIV

Input n (or 0 to end): 199
Roman numeral: CXCIX

Input n (or 0 to end): 2017
Roman numeral: MMXVII

Input n (or 0 to end): 0
Last edited on
closed account (48T7M4Gy)
I don't think the Romans did fractions


@lastchance You'll be pleased to know the Romans apparently did use fractions
https://en.wikipedia.org/wiki/Roman_numerals

(Of course it doesn't alter anything you say about double vs int for this computing exercise and is just as a matter of interest)
lastchance (6980)
Oh no! This is going to be pretty hard coding! Especially since, according to the wikipedia link, their whole numbers use decimal and their fractions duodecimal.

And all my information on Romans cutting things in half comes from Asterix and Obelix!
closed account (48T7M4Gy)
On closer inspection it appears there are only 12 choices and the . and S notation makes some sense :)
jothy (3)
@lastchance Thank you very much for your corrections. I was able to make it work !!!
Topic archived. No new replies allowed.