[Caml-list] there has to be a better way to write this code

[Caml-list] there has to be a better way to write this code

[Chris Hecker]

I'm trying to write a little property system, which means I have a
dictionary/hash with strings for keys and a variant of different types
for the data.  Then, I define get_float and get_string and the like on
this hash, and the functions either return the type if it exists, or
throw an exception if it doesn't.  The attached code below works fine.

My problem is that the attached code is really pretty verbose and
redundant.  Each of the get_* functions has a lot of duplicate code in
it, and I think there must be a better way.  I'd like to write
something like this (not really caml syntax, but you get the idea):

let get_float h s = get_meta h s (Float el -> el)

but I can't see any way to get that to work.  The closest I can come
is this:

let get_meta h s f =
  try
    List.iter f (Hashtbl.find_all h s);
    raise Not_found
  with Match_failure _ -> raise Not_found

exception Found_float of float

let get_float h s =
  try
    get_meta h s (function Float el -> raise (Found_float el))
  with Found_float el -> el

which isn't much of a savings over the code below, and probably isn't
worth it.  (Have I mentioned how annoying that exception at global
scope is? :)

If I didn't want to have multiple types per key, I could do this:

let get_float h s =
  match Hashtbl.find h s with
    Float el -> el
  | _ -> raise Not_found

which is slightly better, but having the multiple types is nice
(especially since Hashtbl supports it).

Thoughts?

Chris


----------------------------------------
type data_type =
    String of string
  | Float of float
  | Vec of (float * float)
  | Bool of bool

let h = Hashtbl.create 7

let _ =
  Hashtbl.add h "foo" (String "foo_string");
  Hashtbl.add h "foof" (Float 3.141);
  Hashtbl.add h "bar" (String "bar_string");
  Hashtbl.add h "bar" (Float 2.718);
  Hashtbl.add h "vec" (Vec (1.0,2.0));
  Hashtbl.add h "yes" (Bool true);
  Hashtbl.add h "no" (Bool false)
    
exception Found_float of float
exception Found_string of string
exception Found_vec of (float * float)
exception Found_bool of bool
    
let get_float h s =
  try
    List.iter (function Float el -> raise (Found_float el) | _ -> ()) (Hashtbl.find_all h s);
    raise Not_found
  with Found_float el -> el

let get_string h s =
  try
    List.iter (function String el -> raise (Found_string el) | _ -> ()) (Hashtbl.find_all h s);
    raise Not_found
  with Found_string el -> el

let get_vec h s =
  try
    List.iter (function Vec el -> raise (Found_vec el) | _ -> ()) (Hashtbl.find_all h s);
    raise Not_found
  with Found_vec el -> el

let get_bool h s =
  try
    List.iter (function Bool el -> raise (Found_bool el) | _ -> ()) (Hashtbl.find_all h s);
    raise Not_found
  with Found_bool el -> el


-------------------
Bug reports: http://caml.inria.fr/bin/caml-bugs  FAQ: http://caml.inria.fr/FAQ/
To unsubscribe, mail caml-list-request@inria.fr  Archives: http://caml.inria.fr

Re: [Caml-list] there has to be a better way to write this code

[Judicaël Courant]

Hi,

On Wed, 27 Jun 2001 20:00:27 -0700
Chris Hecker <checker@d6.com> wrote:

> 
> My problem is that the attached code is really pretty verbose and
> redundant.  Each of the get_* functions has a lot of duplicate code in
> it, and I think there must be a better way.  I'd like to write
> something like this (not really caml syntax, but you get the idea):
> 
> let get_float h s = get_meta h s (Float el -> el)
> 

let get_float h s = get_meta h s (fun (Float el) -> el)

is legal syntax, but IMHO

let get_float h s = get_meta h s (fun x -> match x with Float el -> el | _
-> Not_found)

would be a bit cleaner

Now, what about this for get_meta:

let get_meta h s f =
  let g x = try ignore (f x); true with _ -> false in
  let s = List.find g (Hashtbl.find_all h s) in
  f s
 ;;

Or, if you do not want to apply f twice to its argument, you can rewrite
find to better suit your needs.

The original code is
let rec find p = function
  | [] -> raise Not_found
  | x :: l -> if p x then x else find p l

I suggest:
let rec find_and_apply p = function
  | [] -> raise Not_found
  | x :: l -> try p x with _ -> find_and_apply p l

Then, you can write
let get_meta h s f = find_and_apply f (Hashtbl.find_all h s)

Hope this helps...

Judicaël.
-- 
Judicael.Courant@lri.fr, http://www.lri.fr/~jcourant/
(+33) (0)1 69 15 64 85
-------------------
Bug reports: http://caml.inria.fr/bin/caml-bugs  FAQ: http://caml.inria.fr/FAQ/
To unsubscribe, mail caml-list-request@inria.fr  Archives: http://caml.inria.fr

Re: [Caml-list] there has to be a better way to write this code

[Frederick Smith]

Hi Chris,

Below is my solution.  Its not that different from Judicael Courant's but uses somewhat fewer
exceptions.

Hope this helps.

-Fred


type 'a constructive_bool = True of 'a | False

let rec find f l =
  (match l with
  | [] -> raise Not_found
  | hd::tl -> (match f hd with True a -> a | _ -> find f tl))
  ;;

let gmeta f h s = find f (Hashtbl.find_all h s);;
let gfloat  = gmeta (function Float  el -> True el | _ -> False);;
let gstring = gmeta (function String el -> True el | _ -> False);;
let gvec    = gmeta (function Vec    el -> True el | _ -> False);;
let gbool   = gmeta (function Bool   el -> True el | _ -> False);;


Chris Hecker wrote:

> I'm trying to write a little property system, which means I have a
> dictionary/hash with strings for keys and a variant of different types
> for the data.  Then, I define get_float and get_string and the like on
> this hash, and the functions either return the type if it exists, or
> throw an exception if it doesn't.  The attached code below works fine.
>
> My problem is that the attached code is really pretty verbose and
> redundant.  Each of the get_* functions has a lot of duplicate code in
> it, and I think there must be a better way.  I'd like to write
> something like this (not really caml syntax, but you get the idea):
>
> let get_float h s = get_meta h s (Float el -> el)
>
> but I can't see any way to get that to work.  The closest I can come
> is this:
>
> let get_meta h s f =
>   try
>     List.iter f (Hashtbl.find_all h s);
>     raise Not_found
>   with Match_failure _ -> raise Not_found
>
> exception Found_float of float
>
> let get_float h s =
>   try
>     get_meta h s (function Float el -> raise (Found_float el))
>   with Found_float el -> el
>
> which isn't much of a savings over the code below, and probably isn't
> worth it.  (Have I mentioned how annoying that exception at global
> scope is? :)
>
> If I didn't want to have multiple types per key, I could do this:
>
> let get_float h s =
>   match Hashtbl.find h s with
>     Float el -> el
>   | _ -> raise Not_found
>
> which is slightly better, but having the multiple types is nice
> (especially since Hashtbl supports it).
>
> Thoughts?
>
> Chris
>
> ----------------------------------------
> type data_type =
>     String of string
>   | Float of float
>   | Vec of (float * float)
>   | Bool of bool
>
> let h = Hashtbl.create 7
>
> let _ =
>   Hashtbl.add h "foo" (String "foo_string");
>   Hashtbl.add h "foof" (Float 3.141);
>   Hashtbl.add h "bar" (String "bar_string");
>   Hashtbl.add h "bar" (Float 2.718);
>   Hashtbl.add h "vec" (Vec (1.0,2.0));
>   Hashtbl.add h "yes" (Bool true);
>   Hashtbl.add h "no" (Bool false)
>
> exception Found_float of float
> exception Found_string of string
> exception Found_vec of (float * float)
> exception Found_bool of bool
>
> let get_float h s =
>   try
>     List.iter (function Float el -> raise (Found_float el) | _ -> ()) (Hashtbl.find_all h s);
>     raise Not_found
>   with Found_float el -> el
>
> let get_string h s =
>   try
>     List.iter (function String el -> raise (Found_string el) | _ -> ()) (Hashtbl.find_all h s);
>     raise Not_found
>   with Found_string el -> el
>
> let get_vec h s =
>   try
>     List.iter (function Vec el -> raise (Found_vec el) | _ -> ()) (Hashtbl.find_all h s);
>     raise Not_found
>   with Found_vec el -> el
>
> let get_bool h s =
>   try
>     List.iter (function Bool el -> raise (Found_bool el) | _ -> ()) (Hashtbl.find_all h s);
>     raise Not_found
>   with Found_bool el -> el
>
> -------------------
> Bug reports: http://caml.inria.fr/bin/caml-bugs  FAQ: http://caml.inria.fr/FAQ/
> To unsubscribe, mail caml-list-request@inria.fr  Archives: http://caml.inria.fr

-------------------
Bug reports: http://caml.inria.fr/bin/caml-bugs  FAQ: http://caml.inria.fr/FAQ/
To unsubscribe, mail caml-list-request@inria.fr  Archives: http://caml.inria.fr

Re: [Caml-list] there has to be a better way to write this code

[Chris Hecker]

>Below is my solution.  Its not that different from Judicael Courant's but uses somewhat fewer exceptions.

Ah, for some reason it didn't cross my mind to define my own find function!  I ended up with a hybrid of your two solutions.  I can't decide if the non-exhaustive pattern warning is worse than having to type the 'a option (or your constructive_bool) cases every time:

type data_type =
    String of string
  | Float of float
  | Bool of bool
  | Vector of float * float

let hash_get_meta f h s =
  let rec find f l =
    match l with
      [] -> raise Not_found
    | hd::tl -> 
        try f hd
        with Match_failure _ -> find f tl
  in
  find f (Hashtbl.find_all h s)  
              
let hash_get_string = hash_get_meta (function String el -> el)
let hash_get_bool = hash_get_meta (function Bool el -> el)
let hash_get_float = hash_get_meta (function Float el -> el)
let hash_get_vector = hash_get_meta (function Vector (x,y) -> x,y)

That's pretty close to what I wanted (except for the pattern warnings, dare I ask for a pragma to turn it off locally rather than globally via command line?).  To get rid of them I'd do what you said, but with 'a option instead of defining a new type:

let hash_get_string = hash_get_meta (function String el -> Some el | _ -> None)
let hash_get_float = hash_get_meta (function Float el -> Some el | _ -> None)
let hash_get_bool = hash_get_meta (function Bool el -> Some el | _ -> None)
let hash_get_vector = hash_get_meta (function Vector (x,y) -> Some (x,y) | _ -> None)

Definitely wordier and more redundant, but maybe worth it.

Thanks to all who answered!

Chris


-------------------
Bug reports: http://caml.inria.fr/bin/caml-bugs  FAQ: http://caml.inria.fr/FAQ/
To unsubscribe, mail caml-list-request@inria.fr  Archives: http://caml.inria.fr