On Thu, Jan 09, 2014 at 03:45:54PM +0000, Richard W.M. Jones wrote:
> On Thu, Jan 09, 2014 at 04:21:10PM +0100, Pino Toscano wrote:
> > +  and set_operations op_string =
> > +    let currentopset =
> > +      match (!operations) with
> 
> No need for parentheses around !operations.
> 
> > +        let n = ref op_name in
> > +        let remove = string_prefix op_name "-" in
> > +        if remove then
> > +          n := String.sub op_name 1 (String.length op_name - 1);
> > +        match !n with
> 
> This can be written a bit more naturally as ...
> 
>   let n, remove =
>     if string_prefix op_name "-" then
>       String.sub op_name 1 (String.length op_name-1), true
>     else
>       op_name, false in
>   match n with
>   ...

An even more natural way to write this is:

  let op =
    if string_prefix op_name "-" then
      `Remove (String.sub op_name 1 (String.length op_name-1))
    else
      `Add op_name
   match op with
   | `Add "" | `Remove "" -> (* error *)
   | `Add "defaults" -> Sysprep_operation.add_defaults_to_set opset
   | `Remove "defaults" -> Sysprep_operation.remove_defaults_from_set opset
   | etc

The type of op will be [ `Add of string | `Remove of string ].
Actually you never need to write that type out, as OCaml will infer it
(and check it).  It will only appear in error messages if you get the
code wrong.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to