CODEWITHSUNDEEP
hashocaml
ppaul74
ppaul74

Reputation: 751

Ocaml error with if statement

I have a list of lists, eg [[1;2;3];[2];[3;4;5;6]; [7;8;9;10] I want to place these in a Hashtbl where the key is the length of the list and the value is list of lists, which contains all sublists of the given length.

So for the example above the hash will look like as follows

Key            Value
 1              [[2]]
 3              [[1;2;3]]
 4              [[3;4;5;6];[7;8;9;10]]

In addition I am also trying to keep track of the length of the longest list and that number is what is returned by the function

The code that does this is as follows. 

let hashify lst =
    let hash = Hashtbl.create 123456 in 
        let rec collector curmax lst =
            match lst with 
                    [] -> curmax 
                | h::t -> let len = (List.length h) in 
                                (if ((Hashtbl.mem hash len)=true)
                                then ( let v = (Hashtbl.find hash len) in Hashtbl.add hash len v@[h] ) (* Line 660 *)
                                else ( Hashtbl.add hash len [h]));

                                (collector (max len curmax) t)
        in 
        collector 0 lst
    ;;

Now when I do this I get the following error for the code above

File "all_code.ml", line 600, characters 50-72:
Error: This expression has type unit but an expression was expected of type
     'a list

Why does Ocaml require a return type of 'a list and how do I fix this. Thanks in advance Puneet

Upvotes: 4

Views: 267

Answers (3)

Victor Nicollet
Victor Nicollet

Reputation: 24577

Most heavy work on hash tables in OCaml hugely benefits from an update primitive. There are actually two versions, which do different things depending on whether the value exists in the table. This is the one you wish to use: 

(* Binds a value to the key if none is already present, and then updates
   it by applying the provided map function and returns the new value. *)
let update hashtbl key default func = 
  let value = try Hashtbl.find hashtbl key with Not_found -> default in 
  let value' = func value in
  Hashtbl.remove hashtbl key ; Hashtbl.add hashtbl key value' ; value'

With these primitives, it becomes simple to manage a hashtable-of-lists: 

let prepend hashtbl key item = 
  update hashtbl key [] (fun list -> item :: list)

From there, traversing a list and appending everything to a hashtable is quite simple: 

let hashify lst =
  let hash = Hashtbl.create 607 in 
  List.fold_left (fun acc list -> 
    let l = List.length list in
    let _ = prepend hash l list in 
    max acc l
  ) 0 lst

Upvotes: 4

Thomas
Thomas

Reputation: 5048

You are almost there: @ has a lower priority than apply, and thus, as Basil said, Hashtbl.add hash len v@[h] is parsed as (Hashtbl.add hash len v)@[h]. Moreover, you are using way too much parenthesis and if ((Hashtbl.mem hash len)=true) is unnecessary verbose. So a possible good way to write your function is:

let hashify lst =
  let hash = Hashtbl.create 307 in 
  let rec collector curmax = function
    | [] -> curmax 
    | h::t ->
      let len = List.length h in 
      if Hashtbl.mem hash len then
        let v = Hashtbl.find hash len in
        Hashtbl.add hash len (v@[h])
      else
        Hashtbl.add hash len [h];
      collector (max len curmax) t in 
  collector 0 lst

Upvotes: 4

Basile Starynkevitch
Basile Starynkevitch

Reputation: 1

You probably should add parenthesis in (v@[h]) to avoid have it parsed as (Hashtbl.add hash len v)@[h]

And you probably should not pass 123456 to Hashtbl.create but a reasonable prime number like 307 or 2017

Upvotes: 4

Related Questions