Replace one set of characters with another set of characters

Question

I am writing code to convert Roman Numbers to Decimal Numbers.

I was wondering if this line could be optimised?

String s = "MCMXCIV";
String code = s.replaceAll("I", "a")
               .replaceAll("V", "b")
               .replaceAll("X", "c")
               .replaceAll("L", "d")
               .replaceAll("C", "e")
               .replaceAll("D", "f")
               .replaceAll("M", "g");

Expected value of code: gegceab

EDIT 1: Can the same code can be written in a shorter way?

EDIT 2:

Two conditions I want

Avoid expensive string concatenation
replace each character with corresponding code in O(1)

Does this answer your question? [Replace multiple characters in String with multiple different characters](https://stackoverflow.com/questions/43719805/replace-multiple-characters-in-string-with-multiple-different-characters) — Dang Nguyen, Aug 03 '20 at 02:19

score 1 · Accepted Answer · answered Aug 03 '20 at 02:19

1

I have no idea how this helps you translate roman numbers to decimal numbers, but, sure, this can be simplified!

For starters, .replace does this job strictly better ('replaceAll is a stupid name; the first argument is a regular expression. replace, without the all, also replaces all, and takes an actual raw string, which is what you wanted – java doesn't like breaking backwards compatibility; the silly name is unlikely to be corrected).

Secondly, 'shorter' is mostly meaningless. The relevant goals when writing code are readability and flexibility, and rarely efficiency (performance); shorter is on its own not a relevant goal.

Here's an alternative take; it's significantly more efficient for large strings. For small strings, it's 2020, the time taken is going to round down to 0.

private static final String LETTERS = "IVXLCDM";

char[] cs = s.toCharArray();
for (int i = 0; i < cs.length; i++) {
    cs[i] = 'a' + LETTERS.indexOf(cs[i]);
}
String code = new String(cs);

answered Aug 03 '20 at 02:19

rzwitserloot

85,357
5
51
72

Thank you for answering. I have a question, won't `indexOf` method takes `O(n)`? – Lovesh Dongre Aug 03 '20 at 02:41
`O(n)` where `n` is the constant `7` is `O(1)`. – Stephen C Aug 03 '20 at 02:47
I actually used a HashMap with works fine. I missed the fact that `LETTERS` is final and it will take constant time to search. It also appears to me that using a string is better than using a HashMap in my case. – Lovesh Dongre Aug 03 '20 at 02:55
Update: After using a string runtime changed from `5ms to 3ms` and memory changed to`40.4MB to 39.3MB` – Lovesh Dongre Aug 03 '20 at 03:16
These numbers mean nothing, your input strings are most likely far too short (if they aren't about 10000 characters long you're never going to notice). A HashMap is orders of magnitude slower than an indexOf on a 7-length string. That whole O(n) thing just doesn't apply, whatsoever, until you get into huge numbers. – rzwitserloot Aug 03 '20 at 11:16

score 1 · Answer 2 · answered Aug 03 '20 at 02:22

1

You can use two arrays and loop through them, replacing from[i] with to[i].

String s = "MCMXCIV";
char[] from = new char[]{'I', 'V', 'X', 'L', 'C', 'D', 'M'};
char[] to = new char[]{'a', 'b', 'c', 'd', 'e', 'f', 'g'};

for (int i = 0; i < from.length; i++){
    s = s.replace(from[i], to[i]);
}

answered Aug 03 '20 at 02:22

Badr B

998
1
9
17

You may want to use a `Map` where a key is a `from` character and a value is a `to` character. Then you'll have an easier time keeping consistent data and assuring the 1:1 relation. – akuzminykh Aug 03 '20 at 02:27
@akuzminykh I considered it, but it would actually just complicate the code more without making it any shorter than OP's original code. I think putting the characters in a `char[]` or a `String` is a better option in this case. – Badr B Aug 04 '20 at 04:44

Replace one set of characters with another set of characters

2 Answers2