-1

i am trying to make a function that returns the value of first k elements in a list. This is not working. I do not know why.

Can someone help?

This is the tl function -

let tl j = match j with
  | [] -> []
  | x :: xs -> xs;;

This is the real take function-

let rec take k xs = function
  | [] -> failwith "take"
  | x :: xs -> if k = List.length ([x] @ xs) then [x] @ xs
               else [x] @ take List.length xs (tl(List.rev.xs))
George Willcox
  • 677
  • 12
  • 30
1XCoderX1
  • 27
  • 1
  • 5

3 Answers3

3

The cause for the message is that you left out a pair of parentheses, so you're trying to pass List.length as the first parameter to take.

Another problem is that you're defining a function with three parameters – your definition says that take k xs returns a function that takes a list argument (which is unnamed).

A third problem is take (List.length xs) (tl(List.rev xs)).
This attempts to take one more element than there are from the tail of xs (and, for some reason, in reverse).

So I'm going to rewrite it completely.

First of all, you should not use List.length for this.
(List.length is almost never a good solution.)
You only need to care about whether the list is empty or not, it doesn't matter how long it is.

Now, a more formal definition:

take k xs is

  • if k is 0, it's the empty list
  • otherwise, if xs is the empty list, it's an error
  • otherwise, it is the list whose first element is the head of xs, and whose tail is the result of taking k - 1 elements from the tail of xs.

That is,

let rec take k xs = match k with
    | 0 -> []
    | k -> match xs with
           | [] -> failwith "take"
           | y::ys -> y :: (take (k - 1) ys)
molbdnilo
  • 64,751
  • 3
  • 43
  • 82
3

Your function accepts an integer n, and takes n elements from a list list, which is the list which contains the n first elements of list. So your signature is likely to be:

int -> 'a list -> 'a list = <fun>

You probably already can guess that the function is going to be defined recursively, so you can already write the definition of your function like this:

let rec take n list = ...

You have to build a list, but let's first consider three major classes of values for n:

  • n = 0 : just return an empty list
  • n > 0 : take one element and recurse with n - 1
  • n < 0 : The negative case is easy to miss, but if you are not cautious, you can easily enter an infinite recursion at runtime with invalid inputs. Here, fortunately, we are going to use an exception and make things terminate down the recursive chain (that's what happens in moldbino's answer, for example), but other functions might not behave as nicely. Some implementations of take, when given a negative numbers, try to be useful and takes n elements from the end of the list. Here we will simply treat any n that is not strictly positive as-if if was null.

Null or negative N

When we take zero elements from a list, that means that we return the empty list:

let rec take n list =
  if n > 0
  then ...
  else [];;

Positive N

Now, we consider the case where we take n > 0 elements from a list. Following the recursive definition of lists, we have to consider each possible kind of lists, namely the empty list and non-empty lists.

let rec take n list =
  if n > 0
  then
    match list with
    | [] -> ...
    | x :: xs -> ...
  else [];;

What happens when we want to take n > 0 elements from an empty list? We fail.

let rec take n list =
  if n > 0 then
    match list with
    | [] -> failwith "Not enough elements in list"
    | x :: xs -> ...
  else [];;

Now, the general case of recursion. We have a non-empty list, and n > 0, so we know that we can take at least one element from the list. This value is x, which forms the head of the list we want to return. The tail of the list is the one made of the n-1 elements of xs, which is easily computed by take itself:

let rec take n list =
  if n > 0 then
    match list with
    | [] -> failwith "Not enough elements in list"
    | x :: xs -> x :: (take (n - 1) xs)
  else [];;
Community
  • 1
  • 1
coredump
  • 37,664
  • 5
  • 43
  • 77
2

Your immediate problem is that you need parentheses around (List.length xs). The code also treats an empty input list as an error in all cases, which is not correct. An empty list is a legitimate input if the desired length k is 0.

Update

You also have one extra parameter in your definition. If you say this:

let myfun k xs = function ...

the function myfun has 3 parameters. The first two are named k and xs and the third is implicitly part of the function expression.

The quick fix is just to remove xs.

I think you might be asking for the wrong number of elements in your recursive call to take. Something to look at anyway.

Jeffrey Scofield
  • 65,646
  • 2
  • 72
  • 108
  • let rec take k xs = function |[] -> failwith "take" |x :: xs -> if k = List.length ([x] @ xs) then [x] @ xs else [x] @ take (List.length xs) (tl(List.rev xs)) – 1XCoderX1 Oct 22 '16 at 16:11
  • this error comes up -------- Error: This expression has type 'a list -> 'a list but an expression was expected of type 'a list – 1XCoderX1 Oct 22 '16 at 16:12
  • Your function has an extra parameter in its declaration. See updated answer. – Jeffrey Scofield Oct 22 '16 at 16:14