Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm working on Ocaml.org 99 problems, and I solved the run-length decode one.

Here is the solution given by the site:

let decode l =
    let rec many acc n x = 
            if n = 0 then acc else many (x :: acc) (n-1) x in
    let rec aux acc = function
            | [] -> acc
            | One x :: t -> aux (x :: acc) t
            | Many (n,x) :: t -> aux (many acc n x) t  in
    aux [] (List.rev l);;

And here is mine:

let decode l =
    let rec aux acc = function
            | [] -> acc
            | One elem :: t -> aux (elem :: acc) t
            | Many(cnt, elem) :: t -> if cnt > 0 then aux (elem::acc) (Many(cnt - 1, elem) :: t) else aux acc t in
    List.rev (aux [] l);;

Test:

type 'a rld =
    | One of 'a  
    | Many of int * 'a;;

(* result expected : ["a"; "a"; "a"; "a"; "b"; "c"; "c"; "a"; "a"; "d"; "e"; "e"; "e"; "e"] *)
decode [Many (4,"a"); One "b"; Many (2,"c"); Many (2,"a"); One "d"; Many   (4,"e")];;

Could my solution become more optimal?

share|improve this question

Elegance is a matter of taste, so you'll probably get different answers for that point. Personally, I find your solution easier to understand: it makes good use of the algebra defined through the rld type.

From an efficiency standpoint, your algorithm creates as Many values as the repetition count in the original one. That means that your algorithm will generate memory allocation. It will also have to match it again as many times through recursion, so there's a computational extra load as well. The solution proposed by the site is more efficient in that regard: it only allocates the necessary values to construct the resulting list, and only matches once each element of the input list.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.