Don't return options on Txn.go and Cursor.go

[madroach]

instead pass a custom abort function to the user which will take as argument the value to return after aborting.

[Drup]

What does that gives us ? The API is more complicated, and I don't see any improvements.

I would rather move to returning an ('a, exn) error, in fact.

[madroach]

Hi, thanks for having a look.

I can see why you say this (slightly) complicates the api. But please realise that the complexity in the api is only shifted from the return type to the type of the f argument and from the non aborting case to the aborting case. I think the non aborting case is way more common.

The user code gets a lot simpler when transactions are not aborted, which in my mind is way more common than aborting transactions:

before

let x =
  match
    Txn.go Ro map @@ fun txn ->
    Map.get ~txn "x"
  with
  | None -> assert false
  | Some x -> x
in
match
  Txn.go Ro map @@ fun txn ->
  Map.put ~txn "x" 120;
  if x = 120
  then Txn.abort txn
  else 120
with
| None -> failwith "aborted"
| Some x -> ...

after:

let x =
  Txn.go Ro map @@ fun _ txn ->
  Map.get ~txn "x"
in
match
  Txn.go Ro map @@ fun abort txn ->
  Map.put ~txn "x" 120;
  if x = 120
  then abort None (* could also just directly call failwith here and not use options *)
  else Some 120
with
| None -> failwith "aborted"
| Some x -> ...

Why do you want to use a ('a, exn) error type? What do we have to gain from that?

[madroach]

I would also propose to remove the abort functionality from cursors and ask the user to use a transaction explicitely.

[Drup]

The api is more complicated because it's less standard. option types are well known, everyone understands what they do, and the types are very informative. Your new ('a -> 'a) function is fully magically (actually, it should be 'a -> 'b, which is even more magical), so users basically need to see a code pattern to understand how to use it.

[madroach]

Then what about adding a go_exc ? Or rather rename the existing go to go_abortable ? I find the option return type really annoying because most of the time I just return unit and then need to match on Some () or add ignore.

[madroach]

Well, anyway I can live with the option return type as is. But do you have an idea why the travis builds are failing seemingly randomly ?