-1

I am working on a problem on UVa for general programming practice, as I want to get better at programming competitively. However I am having trouble with this problem - Roman Numerals. In this problem the goal is to take input which will be in the form of either a Roman numeral or Arabic numeral and then I must convert from one to the other. I feel that my code should not have trouble in processing fast enough yet according to the online judge, it does not process fast enough. I need to help finding out how I may optimize my code so that it will run faster and not receive TLE.

Below is my program, any help as to explaining why I am receiving Time Limit Exceeded would be greatly appreciated. Thanks in advance.

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;


class Main {
    private static String order = "IVXLCDM";
    private static String order2 = "IXCM";  // These chars are the result of 10^n (n depending on index in the string)
    private static String order3 = "VLD";   // These chars are products of 5*10^n (n depending on index in the string)

    public static void main(String[] args) {
        InputStreamReader isr = new InputStreamReader(System.in);
        BufferedReader br = new BufferedReader(isr);
        String ans = "";
        while (true) {
            String read = "";
            int aNum = 0;
            String rNum = "";
            try {
                read = br.readLine();
                if (read=="") 
                    break;
            } catch (IOException e) {
                if (read=="")
                    break;
                e.printStackTrace();
            }
            try {
                aNum = Integer.parseInt(read);
//                  System.out.println(aNum);

                int thousands = aNum/1000;
//                  System.out.println(thousands);

                int hundreds = aNum/100;
                hundreds = hundreds%10;
//                  System.out.println(hundreds);

                int tens = aNum%100;
                tens = tens/10;
//                  System.out.println(tens);

                int ones = aNum%10;
//                  System.out.println(ones);           

                rNum+= a2R(thousands,"M");
                rNum+= a2R(hundreds,"C");
                rNum+= a2R(tens,"X");
                rNum+= a2R(ones,"I");

//              System.out.println(rNum);
                ans+=(rNum+"\n");
//              System.out.print(ans);

            } catch (NumberFormatException c) {
                rNum = read;
                if (rNum.equals(""))
                    break;
                aNum = r2A(rNum);
//              System.out.println(aNum);
                ans+=(aNum+"\n");
//              System.out.print(ans);
            }
        }
        System.out.print(ans);


    }

    private static int r2A(String rNum) {
        int aNum = 0;
        for (int i = order.length()-1; i >= 0; i--) {
            char curChar = order.charAt(i);
            while (rNum.indexOf(curChar)!=-1) {
                if (rNum.indexOf(curChar)==0) {
                    if (order2.indexOf(curChar)!=-1) {
                        aNum+=((int)Math.pow(10, order2.indexOf(curChar)));
                    }
                    else if (order3.indexOf(curChar)!=-1) {
                        aNum+=(5*((int)Math.pow(10, order3.indexOf(curChar))));
                    }
                    rNum = rNum.substring(1);
                }
                else if (rNum.indexOf(curChar)==1) {
                    if (order2.indexOf(curChar)!=-1) {
                        aNum+=((int)(Math.pow(10, order2.indexOf(curChar))-Math.pow(10, order2.indexOf(curChar)-1)));
                    }
                    else if (order3.indexOf(curChar)!=-1) {
                        aNum+=((int)((5*Math.pow(10, order3.indexOf(curChar)))-Math.pow(10,order3.indexOf(curChar))));
                    }
                    rNum = rNum.substring(2);
                }
            }
        }
        return aNum;
    }

    private static String a2R(int num, String theNum) {
        // num is the digit of an Arabic digit number to be replaced by Roman Numerals for that digit
        // theNum is the value of Roman Numerals that would go into the specific digit place (tens, ones,...)
        String rNum = "";
        if (!theNum.equals("M")) {
            if (num==9) {
                rNum = theNum + order.charAt(order.indexOf(theNum)+2);
            }
            else if (num==4) {
                rNum = theNum + order.charAt(order.indexOf(theNum)+1);
            }
            else if (num>=5) {
                rNum+= order.charAt(order.indexOf(theNum)+1);
                for (int i = 0; i < num-5; i++) {
                    rNum+=theNum;
                }
            }
            else {
                for (int i = 0; i < num; i++) {
                    rNum+=theNum;
                }
            }
        }
        else {
            for (int i = 0; i < num; i++) {
                rNum+=theNum;
            }
        }
        return rNum;
    }

}

`

Noah
  • 1
  • 2
  • I suggest you avoid using `+=` or `+` on Strings and `Math.pow` or String.indexOf. – Peter Lawrey Jul 07 '14 at 21:01
  • Sad that Java does not provide Integer.TryParse, to avoid catching exceptions for flow control. Look into some of the alternatives for parsing integers without throwing an exception on failure. Also, close your streams when done. – Eric J. Jul 07 '14 at 21:01
  • You solution seems a little verbose, there doesn't seem to be any need to actually do the computation since there are so few Roman numerals. You could just create a HashMap that maps the Roman numeral to their respective arabic numerals and vice versa. There would need to be a little extra logic to have subtraction cases like IV and IX, etc.. – Hunter McMillen Jul 07 '14 at 21:13
  • @PeterLawrey would these changes improve efficiency for the problem? – Noah Jul 07 '14 at 21:14
  • 2
    `read == ""` looks wrong, no matter what. Don't use `==` to compare `Strings`. Try `read.isEmpty()`. – ajb Jul 07 '14 at 21:16
  • That is why I suggested them. They are all expensive operations. – Peter Lawrey Jul 07 '14 at 21:16
  • @ajb I suspect it times out first as this would result in an NPE. – Peter Lawrey Jul 07 '14 at 21:18
  • @HunterMcMillen I didn't think about using a HashMap. That would probably make things a lot easier and faster. It would not require as many loops. – Noah Jul 07 '14 at 21:19
  • @ajb woops, thank you for that catch. That is a little dumb mistake on my part – Noah Jul 07 '14 at 21:26
  • @PeterLawrey okay, I was unaware of this. I guess that just comes with my inexperience in programming. Thank you for making me aware of this. What do you suggest in place of them then? – Noah Jul 07 '14 at 21:28
  • @Noah Use StringBuilder to build a String. Change the iteration so you are only every multiplying by 10. Parse the String so you only look at each character once. – Peter Lawrey Jul 08 '14 at 07:41
  • I decided to change my methodology about solving this problem (I got AC), and I decided to solve this problem by @HunterMcMillen 's suggestion. I used HashMaps in my solution, but thank you, Peter Lawrey, I did not realize that the code I used was so much slower. I will keep what you told me in mind in the future. – Noah Jul 08 '14 at 17:52

2 Answers2

1

I expect the TLE is being caused by your program never terminating.

Currently you have a while (true) loop, which breaks when you see a blank line.
According to the problem however...

The input consists of several lines, each one containing 
either an Arabic or a Roman number n, where 0 < n < 4000.

Nowhere does it state that there will be an extra blank line terminating the input.
So your program will not terminate, forever waiting until an extra blank line has been entered.

Instead of reading your input like this

    while (true) {
        String read = "";
        int aNum = 0;
        String rNum = "";
        try {
            read = br.readLine();
            if (read=="") 
                break;
        } catch (IOException e) {
            if (read=="")
                break;
            e.printStackTrace();
        }
        //etc

try this instead

    String read = "";
    while ((read = br.readLine()) != null) {
        int aNum = 0;
        String rNum = "";
        //etc
azurefrog
  • 10,785
  • 7
  • 42
  • 56
  • He's got it. On EOF readLine will return null. Your code will try to parse it, throw an exception, them immediately go up and readLine null again. repeat. Infinite loop. – Totoro Jul 07 '14 at 22:38
  • @Totoro No that is not what this code is doing. It assigns the result of `readLine()` to `read` then compares its value against `null`. – Hunter McMillen Jul 08 '14 at 12:58
  • Thanks for the suggestion! It definitely helped! – Noah Jul 08 '14 at 17:54
0

I solved my problem by going about it in a different manner, I used a couple of HashMaps to map Roman numeral values to Arabic numeral values and vice versa. I had four helper methods: one would set up the hashmaps, another would convert from Roman numeral to Arabic numeral, and the other two would work together to convert from Arabic numeral to Roman numeral.

The method that converted from Roman to Arabic would go through the string in a for loop starting from the beginning of the string. It would check if the length of the string was greater than one, and if so it would then check if the substring of the first two values are in the Roman to Arabic hashmap. If so, it would then add the value that the Roman numeral value equates to to an int variable. The method would also check substrings of length 1.

In the methods that converted from Arabic to Roman, the input integer would first be analyzed then it would be torn apart into its little pieces. In the first method, four integer values would first be produced: the thousands value, the hundreds value, the tens value, then the ones value. The second method would organize these values into the correct Roman numeral form.

Thanks to everybody who helped me solve this problem, I did not realize some of the mistakes that I made, probably due to my inexperience in programming so this was a great learning experience for myself.

Below is my solution:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.HashMap;


class Main {
private static HashMap<String,Integer> r2A = new HashMap<String,Integer>();
private static HashMap<Integer,String> a2R = new HashMap<Integer,String>();

public static void main(String[] args) throws IOException {
    InputStreamReader isr = new InputStreamReader(System.in);
    BufferedReader br = new BufferedReader(isr);
    makeMaps();
    String read;
    StringBuilder answers = new StringBuilder("");
    while ((read=br.readLine())!=null) {
        int aNum = 0;
        String rNum = "";
        try {
            aNum = Integer.parseInt(read);
            System.out.println(arab2Roman(aNum));
        } catch (NumberFormatException c) {

            rNum = read;

            int ans = roman2Arab(rNum);
            System.out.println(ans);

        }
    }
}

private static int roman2Arab(String rNum) {
    int aNum = 0;
    for (int i = 0; i < rNum.length(); i++) {
        boolean done = false;
        String theNum = rNum.substring(i,i+1);
        if (i < rNum.length()-1) {
            String part = rNum.substring(i, i+2);
            if (r2A.containsKey(part)) {
                aNum+=r2A.get(part);
                i++;
                done = true;
            }
        }
        if (!done) {
            if (r2A.containsKey(theNum)) {
                aNum+=r2A.get(theNum);
            }
        }

    }
    return aNum;
}

private static String arab2Roman(int num) {
    StringBuilder rNum = new StringBuilder("");
    int thousands = num-(num%1000);

    int hundreds = ((num/100)%10)*100;

    int tens = ((num/10)%10)*10;

    int ones = num%10;

    rNum.append(simpleConv(thousands,"thousands"));
    rNum.append(simpleConv(hundreds,"hundreds"));
    rNum.append(simpleConv(tens,"tens"));
    rNum.append(simpleConv(ones,"ones"));

    return rNum.toString();
}

private static String simpleConv(int num, String place) {
    StringBuilder ans = new StringBuilder("");
    int pNum = (place.equals("thousands")) ? 1000 : (place.equals("hundreds")) ? 100 : (place.equals("tens")) ? 10 : 1;
    if (a2R.containsKey(num)) {
        ans.append(a2R.get(num));
    }
    else {
        if (num/pNum>=5) {
            ans.append(a2R.get(5*pNum));
            for (int i = 0; i < ((num/pNum)-5); i++) {
                ans.append(a2R.get(pNum));
            }
        }
        else {
            for (int i = 0; i < num/pNum; i++) {
                ans.append(a2R.get(pNum));
            }
        }
    }
    return ans.toString();
}

private static void makeMaps() {
    // First r2A
    r2A.put("I", 1);
    r2A.put("IV", 4);
    r2A.put("V", 5);
    r2A.put("IX", 9);
    r2A.put("X", 10);
    r2A.put("XL", 40);
    r2A.put("L", 50);
    r2A.put("XC", 90);
    r2A.put("C", 100);
    r2A.put("CD", 400);
    r2A.put("D", 500);
    r2A.put("CM", 900);
    r2A.put("M", 1000);

    // Second a2R
    a2R.put(1, "I");
    a2R.put(4, "IV");
    a2R.put(5, "V");
    a2R.put(9, "IX");
    a2R.put(10, "X");
    a2R.put(40, "XL");
    a2R.put(50, "L");
    a2R.put(90, "XC");
    a2R.put(100, "C");
    a2R.put(400, "CD");
    a2R.put(500, "D");
    a2R.put(900, "CM");
    a2R.put(1000, "M");

}
}
Noah
  • 1
  • 2