3

Recursive function:

let rec listMerge (l1 : 'a list) (l2 : 'a list) =
    if l1.IsEmpty then      l2 
    elif l2.IsEmpty then    l1 
    else                    l1.Head :: l2.Head :: listMerge l1.Tail l2.Tail

Now, unless I am happily mistaken, this does not actually perform tail call, it just may look like it, if not considering that :: is right associative.

Then, I am under impression (from something I read, but couldn't find now) that this can easily be converted to tail recursive by using an extra fun or something.

So, is it possible? Code?

My answer: So, this is how I changed the functions, thanks to answers below:

let listMerge l1 l2 =
    let rec mergeLoop  (l1 : 'a list) (l2 : 'a list) acc =
        if l1.IsEmpty then      (List.rev acc) @ l2 
        elif l2.IsEmpty then    (List.rev acc) @ l1
        else                    mergeLoop l1.Tail l2.Tail (l2.Head :: l1.Head :: acc)
    mergeLoop l1 l2 []
hyde
  • 60,639
  • 21
  • 115
  • 176
  • 4
    I suggest you read about pattern matching. It'll make your co much more readable and clean. – Ramon Snir Oct 31 '12 at 08:53
  • If you mean `match`, I also have version which uses that, though I think this one is more readable, reading it out loud is almost plain English... IL is quite different though, I'm wondering if I should post another question asking which is more efficient. – hyde Oct 31 '12 at 09:17
  • In general pattern matching (in `match`, `let`, `use`, `fun`, `function` etc.) is more pragmatic for functional code. It will also let you do more complex things, that `if`'s just won't. – Ramon Snir Oct 31 '12 at 09:48
  • 2
    "I'm wondering if I should post another question asking which is more efficient". The code you have written here is extremely bad: unsafe and slow. – J D Nov 04 '12 at 12:52
  • @JonHarrop Even though that kind of no-value comment with zero information is trolling 99% of the time, I'll bite. Unsafe how? – hyde Nov 04 '12 at 17:12
  • 1
    @hyde "Unsafe how?" You are using `Head` and `Tail` functions that can raise exceptions when you could be using pattern matching to get the compiler to prove the correctness of your code in this respect. – J D Nov 05 '12 at 10:46
  • @JonHarrop How is `match` throwing a `MatchFailureException` better than `Head` or `Tail` throwing an `ArgumentException`, if there's a bug in the logic? Sorry if I seem stubborn, I'm all for `match` when it provides extra value, I just don't see the extra value *in this case*. – hyde Nov 05 '12 at 11:20
  • 2
    @hyde If there is ever any risk of `match` throwing a `MatchFailureException` then the compiler will emit a warning. In this case, the compiler proves that `match` will never throw an exception so you know at compile time that your code will never ever fail in this way. – J D Nov 05 '12 at 12:07
  • @JonHarrop Ah, well *that* is very good (I'm a "fix all warnings" fundamentalist, who gets rash from duck typing). – hyde Nov 05 '12 at 15:27

5 Answers5

14

As @Ramon suggested, you should use pattern matching for better readability:

let rec listMerge xs ys =
    match xs, ys with
    | [], _ -> ys
    | _, [] -> xs
    | x::xs', y::ys' -> x::y::listMerge xs' ys'

As you can see, two cons constructors (::) are the last operations on listMerge so the function isn't tail-recursive.

You can use an accumulator to obtain results in a tail-recursive way:

let listMerge xs ys =
    let rec loop xs ys acc =
        match xs, ys with
        | [], zs | zs, [] -> (List.rev zs)@acc
        | x::xs', y::ys' -> loop xs' ys' (y::x::acc)
    List.rev (loop xs ys [])

In the function above, the first List.rev call could be avoided if you add a few more patterns to deconstruct two lists until both of them are empty.

In F#, there is a tail-recursive approach using sequence expressions which is along the line of continuation-passing style:

let listMerge xs ys =
    let rec loop xs ys =
        seq {
            match xs, ys with
            | [], zs | zs, [] -> yield! zs
            | x::xs', y::ys' -> 
                yield x
                yield y
                yield! loop xs' ys'
        }
    loop xs ys |> Seq.toList

I like this approach since it is convenient and close to your original formulation.

pad
  • 41,040
  • 7
  • 92
  • 166
  • Accepted this as most comprehensive, though I personally disagree with `match` being more readable in this case (for this definiton of readable: random coder reading the code and understanding it). – hyde Oct 31 '12 at 09:46
  • 9
    I hope that when you get used to F#, you will change your mind. My definition of readability: "Random *F#* coders read the code and understand it immediately." – pad Oct 31 '12 at 09:58
  • Possible improvement for sequence example, does this work: `| [],s | s,[] -> yield! s` instead of separate `yield! ys` and `yield! xs`? And same for tail-recursive version, of course. – hyde Oct 31 '12 at 11:28
  • You're correct. This is an example of advanced stuffs you can do with pattern matching. I updated my answer; the first example is kept as-is to be close to your question. – pad Oct 31 '12 at 11:45
  • Cool. That's starting to be something which makes `match` worthwhile in my eyes, doing stuff which `if` can't do. – hyde Oct 31 '12 at 11:47
  • 1
    It's also common to see the use of pattern matching function syntax to prevent shadowing of variables in match statements. `let rec loop acc = function | [], zs | zs, [] ->` etc – gradbot Nov 02 '12 at 22:16
2

You can accumulate the constructed result in subsequent calls to listMerge and finally return the accumulated result. My F# skills are pretty rusted, but here goes a simple Lisp function.

(defun list-merge (xs ys &optional acc)
  (cond ((< 0 (length xs)) (list-merge (rest xs) ys (cons (first xs) acc)))
        ((< 0 (length ys)) (list-merge xs (rest ys) (cons (first ys) acc)))
        (t acc)))

(list-merge '(1 2 3) '(3 4 5)) ;=> (5 4 3 3 2 1)
(list-merge '() '(1 2))        ;=> (2 1)
(list-merge '() '())           ;=> nil
Volkan Yazıcı
  • 1,530
  • 1
  • 15
  • 28
1

Simple version using an accumulator:

let rec listMerge (l1 : 'a list) (l2 : 'a list) acc =
    if l1.IsEmpty then      (List.rev l2)@acc 
    elif l2.IsEmpty then    (List.rev l1)@acc
    else                    listMerge l1.Tail l2.Tail (l1.Head :: l2.Head :: acc)

I tested this with two million element lists and there was no stack overflow so I am reasonably confident this is tail recursive.

John Palmer
  • 25,356
  • 3
  • 48
  • 67
  • The order of elements is messed up. If you change the last clause to `(l2.Head :: l1.Head :: acc)`, at least you get the reverse order of OP's results. – pad Oct 31 '12 at 09:25
0

I think you have to reuse the original F# code found in F# PowerPack.
In fact, what you need is List.fold2, except the function should not throw an exception SR.listsHadDifferentLengths in case if the list sizes are different, but instead process the remainder of the longer list, like this:

let l1 = ["A1"; "A2"; "A3"; "A4"; "A5"; "A6"; "A7"]
let l2 = ["B1"; "B2"; "B3"; "B4"]

let expectedResult = ["A1"; "B1"; "A2"; "B2"; "A3"; "B3"; "A4"; "B4"; "A5"; "A6"; "A7"]

Here's how we do it:

[<CompiledName("Fold2Tail")>]
let fold2Tail<'T1,'T2,'State> f g1 g2 (acc:'State) (list1:list<'T1>) (list2:list<'T2>) = 
    let f  = OptimizedClosures.FSharpFunc<_,_,_,_>.Adapt(f)
    let g1 = OptimizedClosures.FSharpFunc<_,_,_>.Adapt(g1)
    let g2 = OptimizedClosures.FSharpFunc<_,_,_>.Adapt(g2)
    let rec loop acc list1 list2 =
        match list1, list2 with 
        | [], [] -> acc
        | _,  []  -> g1.Invoke(acc, list1)
        | [], _   -> g2.Invoke(acc, list2)
        | (h1::t1),(h2::t2) -> loop (f.Invoke(acc,h1,h2)) t1 t2
    loop acc list1 list2

g1 and g2 are predicates of type 'State -> 'T1 list -> 'State and 'State -> 'T2 list -> 'State, correspondingly. They tell how to process the remainders of both lists. Note there are two of them, since in general case 'T1 and 'T2 are different types. Yes, it is somewhat overhead, but you can easily reduce it to your needs, sacrificing universality.

Usage:

let ret =
    fold2Tail
        (fun s x y  -> [ yield! s; yield x; yield y ] ) // f
        List.append // g1
        List.append // g2
        []          // 'State
        l1 l2
Be Brave Be Like Ukraine
  • 7,596
  • 3
  • 42
  • 66
0

You should use pattern matching:

let rec merge xs ys =
  match xs, ys with
  | [], xs | xs, [] -> xs
  | x::xs, y::ys -> x::y::merge xs ys

To get tail calls you can use an accumulator:

let merge xs ys =
  let rec loop xys xs ys =
    match xs, ys with
    | [], xs | xs, [] -> List.fold (fun xs x -> x::xs) xs xys
    | x::xs, y::ys -> loop (y::x::xys) xs ys
  loop [] xs ys

or continuation passing style:

let merge xs ys =
  let rec loop xs ys k =
    match xs, ys with
    | [], xs | xs, [] -> k xs
    | x::xs, y::ys -> loop xs ys (fun xys -> k(x::y::xys))
  loop xs ys id
J D
  • 48,105
  • 13
  • 171
  • 274
  • The continuation-passing style secretly constructs a rather larger closure, so it is no better than the non-tail version. – Andrej Bauer Nov 05 '12 at 01:16
  • @AndrejBauer "no better than the non-tail version". The non-tail version crashes with a stack overflow on large inputs where the CPS version does not. Surely that is "better"? – J D Nov 05 '12 at 10:47
  • Ah, the closure must be landing on the heap then? – Andrej Bauer Nov 05 '12 at 13:40
  • @AndrejBauer: Exactly. The closures actually form a linked list on the heap and all calls are in tail position so stack consumption is bounded for arbitrarily long inputs. According to my measurements it is actually slightly faster than the accumulator pattern too... – J D Nov 05 '12 at 14:37
  • _"it is actually slightly faster than the accumulator pattern"_ — That contradicts [my finding](http://stackoverflow.com/a/7199989/162396). – Daniel Nov 09 '12 at 22:50