Avoiding code duplication in F#

I have two snippets of code that tries to convert a float list to a Vector3 or Vector2 list. The idea is to take 2/3 elements at a time from the list and combine them as a vector. The end result is a sequence of vectors.

    let rec vec3Seq floatList =
        seq {
            match floatList with
            | x::y::z::tail -> yield Vector3(x,y,z)
                               yield! vec3Seq tail
            | [] -> ()
            | _ -> failwith "float array not multiple of 3?"
            }

    let rec vec2Seq floatList =
开发者_运维百科        seq {
            match floatList with
            | x::y::tail -> yield Vector2(x,y)
                            yield! vec2Seq tail
            | [] -> ()
            | _ -> failwith "float array not multiple of 2?"
            }

The code looks very similiar and yet there seems to be no way to extract a common portion. Any ideas?

Here's one approach. I'm not sure how much simpler this really is, but it does abstract some of the repeated logic out.

let rec mkSeq (|P|_|) x =
  seq {
    match x with
    | P(p,tail) -> 
        yield p
        yield! mkSeq (|P|_|) tail
    | [] -> ()
    | _ -> failwith "List length mismatch" }

let vec3Seq =
  mkSeq (function
  | x::y::z::tail -> Some(Vector3(x,y,z), tail)
  | _ -> None)

As Rex commented, if you want this only for two cases, then you probably won't have any problem if you leave the code as it is. However, if you want to extract a common pattern, then you can write a function that splits a list into sub-list of a specified length (2 or 3 or any other number). Once you do that, you'll only use map to turn each list of the specified length into Vector.

The function for splitting list isn't available in the F# library (as far as I can tell), so you'll have to implement it yourself. It can be done roughly like this:

let divideList n list = 
  // 'acc' - accumulates the resulting sub-lists (reversed order)
  // 'tmp' - stores values of the current sub-list (reversed order)
  // 'c'   - the length of 'tmp' so far
  // 'list' - the remaining elements to process
  let rec divideListAux acc tmp c list = 
    match list with
    | x::xs when c = n - 1 -> 
      // we're adding last element to 'tmp', 
      // so we reverse it and add it to accumulator
      divideListAux ((List.rev (x::tmp))::acc) [] 0 xs
    | x::xs ->
      // add one more value to 'tmp'
      divideListAux acc (x::tmp) (c+1) xs
    | [] when c = 0 ->  List.rev acc // no more elements and empty 'tmp'
    | _ -> failwithf "not multiple of %d" n // non-empty 'tmp'
  divideListAux [] [] 0 list      

Now, you can use this function to implement your two conversions like this:

seq { for [x; y] in floatList |> divideList 2 -> Vector2(x,y) }
seq { for [x; y; z] in floatList |> divideList 3 -> Vector3(x,y,z) }

This will give a warning, because we're using an incomplete pattern that expects that the returned lists will be of length 2 or 3 respectively, but that's correct expectation, so the code will work fine. I'm also using a brief version of sequence expression the -> does the same thing as do yield, but it can be used only in simple cases like this one.

This is simular to kvb's solution but doesn't use a partial active pattern.

let rec listToSeq convert (list:list<_>) =
    seq {
        if not(List.isEmpty list) then
            let list, vec = convert list
            yield vec
            yield! listToSeq convert list
        }

let vec2Seq = listToSeq (function
    | x::y::tail -> tail, Vector2(x,y)
    | _ -> failwith "float array not multiple of 2?")

let vec3Seq = listToSeq (function
    | x::y::z::tail -> tail, Vector3(x,y,z)
    | _ -> failwith "float array not multiple of 3?")

Honestly, what you have is pretty much as good as it can get, although you might be able to make a little more compact using this:

// take 3 [1 .. 5] returns ([1; 2; 3], [4; 5])
let rec take count l =
    match count, l with
    | 0, xs -> [], xs
    | n, x::xs -> let res, xs' = take (count - 1) xs in x::res, xs'
    | n, [] -> failwith "Index out of range"

// split 3 [1 .. 6] returns [[1;2;3]; [4;5;6]]
let rec split count l =
    seq { match take count l with
          | xs, ys -> yield xs; if ys <> [] then yield! split count ys }

let vec3Seq l = split 3 l |> Seq.map (fun [x;y;z] -> Vector3(x, y, z))
let vec2Seq l = split 2 l |> Seq.map (fun [x;y] -> Vector2(x, y))

Now the process of breaking up your lists is moved into its own generic "take" and "split" functions, its much easier to map it to your desired type.