1

I am having trouble with my code. Help please. Here is what I have my code so far and I need to use methods. It needs to be able to take an integer 1-3999 and turn that into a roman numeral. Is there an easier way to do this than what I have done?

    public static void main(String[] args) {
    Scanner in = new Scanner (System.in);
    System.out.print("Enter a number between 1 and 3999 (0 to quit): ");
    int input = in.nextInt();
    while (input !=0 )
    {
        if(input < 0 || input > 3999){
        System.out.println("ERROR! NUmber must be between 1 and 3999 (0 to quit): ");
        System.out.print("Enter a number between 1 and 3999 (0 to quit): ");
        input = in.nextInt();
        }
        else if(input > 0){
        String roman = convertNumberToNumeral(input);
        System.out.println("The number " + input + " is the Roman numberal " + roman);
        System.out.print("Enter a number between 1 and 3999 (0 to quit): ");
        input = in.nextInt();
        }
    }
    while (input == 0)
    {
        break;
    }
        System.out.println("Goodbye!");
}

// Given a Scanner as input, prompts the user to input a number between 1 and 3999.
// Checks to make sure the number is within range, and provides an error message until
// the user provides a value within range.  Returns the number input by the user to the
// calling program.
private static int promptUserForNumber(Scanner inScanner, int input) {
}

// Given a number as input, converts the number to a String in Roman numeral format, 
// following the rules in the writeup for Lab 09.  Returns the String to the calling
// program.  NOTE:  This method can possibly get long and complex.  Use the 
// convertDigitToNumeral method below to break this up and make it a bit simpler to code.
private static String convertNumberToNumeral(int input) {
    String romanOnes = ("");
    String romanTens = ("");
    String romanHundreds = ("");
    String romanThousands = ("");
    int ones = input % 10;
    int tens2 = input / 10;
    if (tens2 < 10)
    {
        tens2 = input / 10;
    }
    else {
        tens2 = tens2 % 100;
    }
    int tens = tens2;

    int hundreds2 = input / 100;
    if (hundreds2 < 10)
    {
        hundreds2 = input / 10;
    }
    else {
        hundreds2 = hundreds2 % 1000;
    }

    int hundreds = hundreds2;

    int thousands2 = input / 1000;
    if (thousands2 < 10)
    {
        thousands2 = input / 10;
    }
    else {
        thousands2 = thousands2 % 10000;
    }

    int thousands = input & 10000;
    {
        if (ones == 0)
        {
            romanOnes = ("");
        }
        else if (ones == 1)
        {
            romanOnes = ("I");
        }
        else if (ones == 2)
        {
            romanOnes = ("II");
        }
        else if(ones == 3)
        {
            romanOnes = ("III");
        }
        else if(ones == 4)
        {
            romanOnes = ("IV");
        }
        else if(ones == 5)
        {
            romanOnes = ("V");
        }
        else if(ones == 6)
        {
            romanOnes = ("VI");
        }
        else if(ones == 7)
        {
            romanOnes = ("VII");
        }
        else if(ones == 8)
        {
            romanOnes = ("VIII");
        }
        else if(ones == 9)
        {
            romanOnes = ("IX");
        }
    }
    {
        if (tens == 0)
        {
            romanTens = ("");
        }
        else if (tens == 1)
        {
            romanTens = ("X");
        }
        else if (tens == 2)
        {
            romanTens = ("XX");
        }
        else if(tens == 3)
        {
            romanTens = ("XXX");
        }
        else if(tens == 4)
        {
            romanTens = ("XL");
        }
        else if(tens == 5)
        {
            romanTens = ("L");
        }
        else if(tens == 6)
        {
            romanTens = ("LX");
        }
        else if(tens == 7)
        {
            romanTens = ("LXX");
        }
        else if(tens == 8)
        {
            romanTens = ("LXXX");
        }
        else if(tens == 9)
        {
            romanTens = ("XC");
        }
    }
    {
        if (hundreds == 0)
        {
            romanHundreds = ("");
        }
        else if (hundreds == 1)
        {
            romanHundreds = ("C");
        }
        else if (hundreds == 2)
        {
            romanHundreds = ("CC");
        }
        else if(hundreds == 3)
        {
            romanHundreds = ("CCC");
        }
        else if(hundreds == 4)
        {
            romanHundreds = ("CD");
        }
        else if(hundreds == 5)
        {
            romanHundreds = ("D");
        }
        else if(hundreds == 6)
        {
            romanHundreds = ("DC");
        }
        else if(hundreds == 7)
        {
            romanHundreds = ("DCC");
        }
        else if(hundreds == 8)
        {
            romanHundreds = ("DCCC");
        }
        else if(hundreds == 9)
        {
            romanHundreds = ("CM");
        }
    }
    {
        if (thousands == 0)
        {
            romanThousands = ("");
        }
        else if (thousands == 1)
        {
            romanThousands = ("M");
        }
        else if (thousands == 2)
        {
            romanThousands = ("MM");
        }
        else if(thousands == 3)
        {
            romanThousands = ("MMM");
        }
    }
    String roman = (romanThousands + romanHundreds + romanTens + romanOnes);
    return roman;

}

// Given a digit and the Roman numerals to use for the "one", "five" and "ten" positions,
// returns the appropriate Roman numeral for that digit.  For example, if the number to
// convert is 49 we would call convertDigitToNumeral twice.  The first call would be:
//     convertDigitToNumeral(9, 'I','V','X')
// and would return a value of "IX".  The second call would be:
//     convertDigitToNumeral(4, 'X','L','C')
// and would return a value of "XL".  Putting those togeter we would see that 49 would be the
// Roman numeral XLIX.
// Call this method from convertNumberToNumeral above to convert an entire number into a 
// Roman numeral.
private static String convertDigitToNumeral(int digit, char one, char five, char ten) {


}
user2509405
  • 87
  • 2
  • 2
  • 6
  • 2
    When you get lots of else if's, consider using a switch statement instead (it improves readability, especially when several cases give the same answer). – keyser Jul 04 '13 at 23:45
  • 2
    FYI there are several good Java solutions in the answers to this question: [Converting Integers to Roman Numerals - Java](http://stackoverflow.com/questions/12967896/converting-integers-to-roman-numerals-java) – IgnisErus Jul 04 '13 at 23:50

4 Answers4

7

Wow! You seem to be making this way too complicated. There's a very similar problem on 4clojure.com: Write Roman Numerals. You have a bit of extra error-checking logic here, but even so you shouldn't need nearly this much code. I solved the problem with a 10-line function in Clojure. However, given that most people aren't very comfortable with Lisps, I'll give you a Python solution that I quickly translated from my Clojure solution.

def to_roman(n):
  digits = [(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')]
  result = ""
  while len(digits) > 0:
    (val, romn) = digits[0] # Unpacks the first pair in the list
    if n < val:
      digits.pop(0) # Removes first element
    else:
      n -= val
      result += romn
  return result

Python's syntax is enough like pseudo-code that you can probably understand it even if you don't actually know Python. I'll leave it to you to translate this to Java.


Responding to your comment:

I think my problem is using the methods, how I do it using the methods I have listed?

The comments on your methods are very clear about what they're supposed to do. You need to move all the code using the Scanner from main into promptUserForNumber. You'll call promptUserForNumber from main to get a valid input number.

Once you have the number, you pass it to convertNumberToNumeral to handle the conversion. convertNumberToNumeral should loop over each digit of the number and call convertDigitToNumeral to convert each digit to the corresponding Roman Numeral string. After concatenating all the digit components together convertNumberToNumeral can return the complete Roman Numeral representation string as the result.

The logic of convertDigitToNumeral can be almost identical to the to_roman solution I posted above, but you only need to handle a single digit. It would look something like this:

def digit2roman(n, ten, five, one):
  digits = [(9, one+ten), (5, five), (4, one+five), (1, one)]
  result = ""
  while len(digits) > 0:
    val, romn = digits[0]
    if n < val:
      digits.pop(0)
    else:
      n -= val
      result += romn
  return result

Splitting digits into 2 lists might make this a little easier to convert to Java:

def digit2roman(n, ten, five, one):
  digitsV = [9, 5, 4, 1]
  digitsR = [one+ten, five, one+five, one]
  result = ""
  i = 0
  while i < len(digitsV):
    if n < digitsV[i]:
      i += 1
    else:
      n -= digitsV[i]
      result += digitsR[i]
  return result

In higher-level languages it's common to zip two sequences into a single sequence of pairs when you want to iterate over 2 sequences at the same time, but in Java you usually just iterate over the indices. You should have a simple time of translating this code using Java arrays.

DaoWen
  • 32,589
  • 6
  • 74
  • 101
1
static HashMap<Character, Integer> numToInt;

public static void setup()
{
    numToInt = new HashMap<Character, Integer>();

// this is my trick to avoid listing the numbers
    String numerals = "MDCLXVI";
    int number = 1000;
    int factor = 2;
    for (char numeral : numerals.toCharArray()) {
        numToInt.put(numeral, number);
        number /= factor;
        factor = (factor == 2 ? 5 : 2);
    }
}

public static int romanNumeralsToInt(String numeralStr)
{
    int total = 0;
    int prevVal = 0;
    for (int i = 0; i < numeralStr.length(); i++) {
        int val = numToInt.get(numeralStr.charAt(i));
        total += val - (prevVal > val && prevVal != 0 ? 0 : 2 * prevVal);
        prevVal = val;
    }
    int len = numeralStr.length();
    return total;
}
isaach1000
  • 1,819
  • 1
  • 13
  • 18
  • Just by looking at this, I think something is wrong. You set `I` to 1000, V to 500, X to 100, L to 50 (correct), C to 10, D to 5 and M to 1. –  Jul 05 '13 at 00:12
  • I think you want to start with `number = 1000, factor = 5`, then multiply by the factor instead of dividing. –  Jul 05 '13 at 00:13
  • Sorry I listed the letters backwards. Switched now. Thank you @DaftPunk. – isaach1000 Jul 05 '13 at 00:14
1

@DaoWen's answer is very good, but since there are so few cases for the convertDigitToNumeral method to handle you could just hard code all of them. Here's a simple solution using Java's format strings (using positional arguments):

private static final String[] digitFormats = new String[] {
  "", "%1$c", "%1$c%1$c", "%1$c%1$c%1$c", "%1$c%2$c",
  "%2$c", "%2$c%1$c", "%2$c%1$c%1$c", "%2$c%1$c%1$c%1$c", "%2$c%3$c"
};

private static String convertDigitToNumeral(int digit, char one, char five, char ten) {
  return String.format(digitFormats[digit], one, five, ten);
}

The format string "%2$c%1$c%1$c" is the same as saying five.toString() + one.toString() + one.toString() (assuming the format arguments are passed in the order one, five, ten).

IgnisErus
  • 386
  • 1
  • 5
0

Is there an easier way?

As already suggested you can use a HashTable as a lookup for the roman codes. You can either have a larger look-up with all ten options each for ones, ten and hundreds. Or use a bit of arithmetic as in the python code in the answer to limit it to just the required ones.

If you just want your code to be more readable:

  1. Condense your calculations.
  2. Convert your nested ifs to a switch statement.
string result = "";

int ones = input % 10;
int tens = (input /10) % 10;
int hundreds = (input / 100) % 10;
int thousands = input / 1000;



switch (thousands)
{
   case 1:
       result = "M"
       break;
   case 2:
       result = "MM"
       break;

// etc ...
}

switch (tens)
{
   case 1:
       result = result + "C"
       break;
// etc ..
}

switch (hundreds)
{
// etc ..
}

switch (ones)
{
// etc ..
}

return result;

Eli Algranti
  • 8,707
  • 2
  • 42
  • 50
  • I'd not switch (thousands) but rather new String(new char[thousands]).replace('\0','M'); This first creates a string of empty (\0) chars and then replaces them all with an M. Same for the others – Xesau May 09 '15 at 23:09
  • @Xesau, there are countless better ways to tackle this problem (some already suggested). As mentioned in my answer the use of switch statements was a suggestion to "make the OP's code more readable" without changing the logic. – Eli Algranti May 11 '15 at 00:41