-2

I'm trying to implement a C program which converts an integer n (between 0 and 1024) into a Roman numeral by way of a greedy algorithm. I have tried to do this in the following manner:

#include <stdio.h>
#include <string.h>
void convert(int);
int max(int[], int, int);

int main(){

    //User Input
    int n;
    printf("Enter a digit between 0 and 1024: ");
    scanf("%d", &n);

    //Validation
    while((n < 0)||(n > 1024)){
        printf("That number is not between 0 and 1024. Please try again: ");
        scanf("%d", &n);
    }

    //Output
    printf("\nAs a Roman numeral, this was written: ");
    if (n == 0) printf("nulla");    //Romans wrote 'nulla' for zero
    else convert(n);

    return 0;
}

void convert(int n){
    //Case n = 0
    if (n == 0)  return;

    else{
        //Case n > 0
        char* romanNums[] = {"I", "IV", "V", "IX", "X", "XL", "L", "XC", "C", "CD", "D", "CM", "M"};
        int arabicNums[] = {1, 4, 5, 9, 10, 40, 50, 90, 100, 400, 500, 900, 1000};
        int biggestNo = max(arabicNums, 12, n); //Biggest number in arabicNums[] smaller than n

        printf("%s", romanNums[biggestNo]); //Print the biggest number as a Roman numeral
        convert(n - biggestNo);             //Convert the remaining

    }
}

//This function determines the maximum number in arr[] smaller than n
int max(int arr[], int size, int n){
    int i, max;
    for(i = 0; i < size; i++){
        if (n < arr[i]) max = i;
    }
    return max;
}

I have tried debugging and modifying aspects of the code, but it's not working. I'd appreciate any feedback.

UPDATE I've managed to amend my program so that it outputs the values 1, 4, 5 etc. correctly, but composite values (i.e. those which require another iteration of convert()) keeps resulting in "Romans.exe not responding". Here is the new code:

#include <stdio.h>
#include <string.h>
void convert(int);
int max(int[], int, int);

int main(){

    //User Input
    int n;
    printf("Enter a digit between 0 and 1024: ");
    scanf("%d", &n);

    //Validation
    while((n < 0)||(n > 1024)){
        printf("That number is not between 0 and 1024. Please try again: ");
        scanf("%d", &n);
    }

    //Output
    printf("\nAs a Roman numeral, this was written: ");
    if (n == 0) printf("nulla");    //Romans wrote 'nulla' for zero
    else convert(n);

    return 0;
}

void convert(int n){
    //Case n = 0
    if (n == 0)  return;

    else{
        //Case n > 0
        char* romanNums[] = {"I", "IV", "V", "IX", "X", "XL", "L", "XC", "C", "CD", "D", "CM", "M"};
        int arabicNums[] = {1, 4, 5, 9, 10, 40, 50, 90, 100, 400, 500, 900, 1000};
        int biggestNo = max(arabicNums, 13, n); //Biggest number in arabicNums[] smaller than n

        printf("%s", romanNums[biggestNo]); //Print the biggest number as a Roman numeral
        convert(n - arabicNums[biggestNo]); //Convert the remaining

    }
}

//This function determines the maximum number in arr[] smaller than n
int max(int arr[], int size, int n){
    int i, max;
    for(i = size; i > 0; i--){
        if (n <= arr[i]) max = i;
    }
    return max;
}
Luke Collins
  • 1,433
  • 3
  • 18
  • 36
  • 1
    Can you be more specific about how it is not working? –  Mar 21 '16 at 21:15
  • @MartinBroadhurst I don't think it's terminating - when I run the program stops responding. – Luke Collins Mar 21 '16 at 21:16
  • I think the `while` loop is your problem (in `main`). `n` never changes in the body of that loop, so you'll never leave it in cases where `n` is outside the range you are looking for. – Jake Smith Mar 21 '16 at 21:18
  • If a bug causes `n` to become negative, the program will never end. In other words, the code checks for `n==0`, and the comment in the `else` says `n>0`, but nothing checks that `n` is *actually* `>0`. – user3386109 Mar 21 '16 at 21:18
  • @MartinBroadhurst Nothing, it doesn't run :/ – Luke Collins Mar 21 '16 at 21:23
  • @user3386109 So I can fix that by another validation check right? But what about the algorithm itself? Is it correct? – Luke Collins Mar 21 '16 at 21:24
  • 1
    @LukeCollins My comment was more of a debugging hint: you could put in an extra `printf("%d\n",n);` at the beginning of the function to see what's happening with `n`. OTOH, in the final production code, I would change the first `if` to `if (n<=0)`. That would terminate the recursion if/when something goes wrong. – user3386109 Mar 21 '16 at 21:35
  • In response to the edit: the loop in `max` should be `for (i = size-1; i >= 0; i--)`. Array indexes go from `0` to `< size` or from `size-1` to `0`. – user3386109 Mar 21 '16 at 22:04

2 Answers2

3

You have 2 missing point on your code:

On function max:

for(i = 0; i < size; i++){
    if (n < arr[i]) max = i;
}

must be:

for(i = 0; i <= size; i++){
    if (n >= arr[i]) max = i;   // Equal is require. Isn't it?
}

On your main function: function convert:

convert(n - biggestNo);             //Convert the remaining

must be:

convert(n - arabicNums[biggestNo]);

biggestNo is seq number, not is value for div

cuong hoang
  • 94
  • 5
1

You are passing a size of 12 to the max function, when the size of the arabicNums array is 13.

Leinadp
  • 36
  • 1