Help me improve this Erlang?

Question

So, I'm really interested in Erlang. I cannot justify using it for something big, but from time to time I try to use it for toy problems.

I am currently using the Roman Numeral translator. I'm just doing an on now, and I find the code terribly duplicate. It works like a charm, but, well, just look at it:

-module(roman). -compile([export_all]). toRoman(N) -> toRoman(N, []). toRoman(0,Acc) -> lists:reverse(lists:flatten(Acc)); toRoman(N, Acc) when N >= 1000 -> toRoman(N-1000,["M"|Acc]); toRoman(N,Acc) when N >= 900 -> toRoman(N-900,["CM" | Acc]); toRoman(N,Acc) when N >= 500 -> toRoman(N-500,["D" | Acc]); toRoman(N,Acc) when N >= 400 -> toRoman(N-400, ["CD" | Acc]); toRoman(N,Acc) when N >= 100 -> toRoman(N-100, ["C" | Acc]); toRoman(N,Acc) when N >= 90 -> toRoman(N-90, ["XC" | Acc]); toRoman(N,Acc) when N >= 50 -> toRoman(N-50, ["L" | Acc]); toRoman(N, Acc) when N >= 40 -> toRoman(N-40, ["XL" | Acc]); toRoman(N, Acc) when N >= 10 -> toRoman(N-10, ["X" | Acc]); toRoman(N, Acc) when N >= 9 -> toRoman(N-9, ["IX" | Acc]); toRoman(N, Acc) when N >= 5 -> toRoman(N-5, ["V" | Acc]); toRoman(N, Acc) when N >= 4 -> toRoman(N-4, ["IV" | Acc]); toRoman(N, Acc) -> toRoman(N-1, ["I" | Acc]). test() -> Test = fun(X) -> io:format("~p -> ~p~n", [X, toRoman(X)]) end, lists:map(Test, [0,1,3,6,23,43,75,87,13,23, 3999, 3998, 2531, 140]). 

I feel that there is a much better way to do this. Can someone give an idea?

Answer 1

If you want to not repeat yourself, you can inspire my Code Golf New Year Edition - An integer up to Roman numeral contribution .

 -module(n2). -export([y/1]). -define(D(V,S),n(N)when N>=V->[??S|n(NV)];). y(N)->io:format(n(N)). ?D(1000,M)?D(900,CM)?D(500,D)?D(400,CD)?D(100,C)?D(90,XC)?D(50,L)?D(40,XL)?D(10,X)?D(9,IX)?D(5,V)?D(4,IV)?D(1,I)n(0)->[10]. 

It is not recommended and recommended to write code in erlang. Macros are bad. Avoid this if possible. It is difficult to debug, it introduces dependencies between intermodules that are not tracked by exchanging hot code, and so on. If you like the more functional approach “code is data, data is code”, look at this as an example:

 -module(roman). -compile([export_all]). toRoman(N) when is_integer(N), N >= 0 -> toRoman(N, [{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"}]). toRoman(0, _) -> []; toRoman(N, [{X, V} | _] = S) when N >= X -> [V | toRoman(N - X, S)]; toRoman(N, [_ | S]) -> toRoman(N, S). test() -> F = fun (X) -> lists:flatten(toRoman(X)) end, "" = F(0), "I" = F(1), "III" = F(3), "VI" = F(6), "XXIII" = F(23), "XLIII" = F(43), "LXXV" = F(75), "LXXXVII" = F(87), "XIII" = F(13), "XXIII" = F(23), "MMMCMXCIX" = F(3999), "MMMCMXCVIII" = F(3998), "MMDXXXI" = F(2531), "CXL" = F(140), ok. 

Just for curiosity, your code is about 5% faster in bytecode and 5% slower than mine. It performs one translation at 1.2us in bytecode and at 370 ns on the integrated Intel (R) Core (TM) 2 Duo T7500 @ 2.20GHz processor.

Change I did not use the recursive version of the tail, because the depth of the recursion is very small. Therefore, I was curious if he had any penalties or benefits from this. I can’t measure any in my bytecode algorithm even my own, but an interesting thing happens in the source code. If I wrote the original algorithm in a direct way (not optimized for tail calling), it is about 40% faster than mine in native code (one conversion in about 250 ns). There is no noticeable difference in the byte code. An interesting example is that tail call optimization is not worth doing.

 toRoman(N) when N >= 1000 -> "M" ++ toRoman(N - 1000); toRoman(N) when N >= 900 -> "CM" ++ toRoman(N - 900); toRoman(N) when N >= 500 -> "D" ++ toRoman(N - 500); toRoman(N) when N >= 400 -> "CD" ++ toRoman(N - 400); toRoman(N) when N >= 100 -> "C" ++ toRoman(N - 100); toRoman(N) when N >= 90 -> "XC" ++ toRoman(N - 90); toRoman(N) when N >= 50 -> "L" ++ toRoman(N - 50); toRoman(N) when N >= 40 -> "XL" ++ toRoman(N - 40); toRoman(N) when N >= 10 -> "X" ++ toRoman(N - 10); toRoman(N) when N >= 9 -> "IX" ++ toRoman(N - 9); toRoman(N) when N >= 5 -> "V" ++ toRoman(N - 5); toRoman(N) when N >= 4 -> "IV" ++ toRoman(N - 4); toRoman(N) when N >= 1 -> "I" ++ toRoman(N - 1); toRoman(0) -> []. 

PS . List antialiasing is not common with Erlang code. The return structure in the above examples is well known as io_list and is commonly accepted in the erlang io system. You can send it directly to sockets, ports, and so on. If you want to write it, for example, you can use io:put_chars(IOList) or io:format("Result: '~s'~n", [IOList]) .

EDIT2 . If the constant list is like the left operand ++ , the erlang compiler operator optimizes the list concatenation for you, therefore ["string" | L] ["string" | L] not required for speed. The resulting code is more readable, and the result is smoothed without penalty for performance. Personnel, if I am interested in performace, I would use this version, which is a bit repetitive, but the fastest that I know, and performs one conversion in 310ns in byte code and in 210ns in native.

 toRoman(N) when N >= 1000 -> "M" ++ toRoman(N - 1000); toRoman(N) -> toRomanC(N div 100, N rem 100). toRomanC(0, N) -> toRomanX(N); toRomanC(1, N) -> "C" ++ toRomanX(N); toRomanC(2, N) -> "CC" ++ toRomanX(N); toRomanC(3, N) -> "CCC" ++ toRomanX(N); toRomanC(4, N) -> "CD" ++ toRomanX(N); toRomanC(5, N) -> "D" ++ toRomanX(N); toRomanC(6, N) -> "DC" ++ toRomanX(N); toRomanC(7, N) -> "DCC" ++ toRomanX(N); toRomanC(8, N) -> "DCCC" ++ toRomanX(N); toRomanC(9, N) -> "CM" ++ toRomanX(N). toRomanX(N) -> toRomanX(N div 10, N rem 10). toRomanX(0, N) -> toRomanI(N); toRomanX(1, N) -> "X" ++ toRomanI(N); toRomanX(2, N) -> "XX" ++ toRomanI(N); toRomanX(3, N) -> "XXX" ++ toRomanI(N); toRomanX(4, N) -> "XL" ++ toRomanI(N); toRomanX(5, N) -> "L" ++ toRomanI(N); toRomanX(6, N) -> "LX" ++ toRomanI(N); toRomanX(7, N) -> "LXX" ++ toRomanI(N); toRomanX(8, N) -> "LXXX" ++ toRomanI(N); toRomanX(9, N) -> "XC" ++ toRomanI(N). toRomanI(0) -> []; toRomanI(1) -> "I"; toRomanI(2) -> "II"; toRomanI(3) -> "III"; toRomanI(4) -> "IV"; toRomanI(5) -> "V"; toRomanI(6) -> "VI"; toRomanI(7) -> "VII"; toRomanI(8) -> "VIII"; toRomanI(9) -> "IX". 

Answer 2

Actually your code is not repeated. It just looks like this because the text forms a repeating pattern. But each of your sentences handles a specific case with a slight logical overlap between them. You can override the switch statement, but you will get a similar repetition. There are too many cases in Roman digital translation, and I do not think that you can avoid the repetitive feeling that makes each of these individual decisions.

Answer 3

I added this link to rosettacode.org before, reselling it here. I found the solution quite elegant.

 -module(roman). -export([to_roman/1]). to_roman(0) -> []; to_roman(X) when X >= 1000 -> "M" ++ to_roman(X-1000); to_roman(X) when X >= 100 -> digit(X div 100, $C,$D,$M) ++ to_roman(X rem 100); to_roman(X) when X >= 10 -> digit(X div 10, $X,$L,$C) ++ to_roman(X rem 10); to_roman(X) when X >= 1 -> digit(X, $I,$V,$X). digit(1,X,_,_) -> [X]; digit(2,X,_,_) -> [X,X]; digit(3,X,_,_) -> [X,X,X]; digit(4,X,Y,_) -> [X,Y]; digit(5,_,Y,_) -> [Y]; digit(6,X,Y,_) -> [Y,X]; digit(7,X,Y,_) -> [Y,X,X]; digit(8,X,Y,_) -> [Y,X,X,X]; digit(9,X,_,Z) -> [X,Z]. 

Answer 4

The repeating part is the accumulation and invocation of a function. Moving them to one place and everything will look much better.

 %%% Roman numerals ruleset r(N) when N >= 1000 -> {N-1000, "M"}; r(N) when N >= 900 -> {N-900, "CM"}; r(N) when N >= 500 -> {N-500, "D"}; r(N) when N >= 400 -> {N-400, "CD"}; r(N) when N >= 100 -> {N-100, "C"}; r(N) when N >= 90 -> {N-90, "XC"}; r(N) when N >= 50 -> {N-50, "L"}; r(N) when N >= 40 -> {N-40, "XL"}; r(N) when N >= 10 -> {N-10, "X"}; r(N) when N >= 9 -> {N-9, "IX"}; r(N) when N >= 5 -> {N-5, "V"}; r(N) when N >= 4 -> {N-4, "IV"}; r(N) when N > 0 -> {N-1, "I"}. roman(N, Acc) -> case r(N) of {0, R} -> [R | Acc]; {N2, R} -> roman(N2, [R | Acc]) end. roman(N) -> list_to_binary(lists:reverse(roman(N, ""))). 

By the way, you get the same result for 4 and 6:

 8> [roman:toRoman(N) || N <- lists:seq(1,10)]. ["I","II","III","VI","V","VI","VII","VIII","XI","X"] 

The same error gives you that 9 and 11 are equal, and 40 and 60, 90 and 110 ....

Answer 5

This process consists of three parts: a list of rules for which symbols represent numbers, a search by these rules to search for the next symbol, and an iteration that reduces the number to zero. Each part receives a function, and we have:

 ruleset() -> [ {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"}]. find_next(N) -> find_next(ruleset(), N). find_next([{V, Symbol}|_], N) when N >= V -> {N - V, Symbol}; find_next([_|T], N) -> find_next(T, N). roman(N, Acc) -> case find_next(N) of {0, R} -> [R | Acc]; {N2, R} -> roman(N2, [R | Acc]) end. roman(N) -> lists:append(lists:reverse(roman(N, ""))). 

Perhaps you could use the lists: foldl / 3 to simplify this a bit more.

Answer 6

This is not repeated, since the "logic" must be implemented in any case. One thing you could do is make it not tail recursive, since you won't have more than 20-30 recursions ...

 -module(roman). -compile([export_all]). to_roman(N) when N >= 1000 -> "M" ++ to_roman(N-1000); to_roman(N) when N >= 900 -> "CM" ++ to_roman(N- 900); ... to_roman(N) when N >= 4 -> "IV" ++ to_roman(N- 4); to_roman(N) when N >= 1 -> "I" ++ to_roman(N- 1); to_roman(_) -> []. 

You can save some characters by specifying a macro. I hate macros, but you might like them :).

 -module(roman). -compile([export_all]). -define( TO_ROMAN(L, C) , to_roman(N) when N >= L -> C ++ to_roman(NL) ). ?TO_ROMAN(1000, "M"); ?TO_ROMAN( 900, "CM"); ... ?TO_ROMAN( 4, "IV"); ?TO_ROMAN( 1, "I"); to_roman(_) -> []. 

Answer 7

This gets a little bolder if you support all variations of Roman numerals, like in Excel, but basically your code stays close to the big headings / matching patterns ...

Answer 8

Using a lookup table should make it shorter and faster in any language.