6
\$\begingroup\$

I have a piece of code that works but, as I start learning OCaml, I am not confortable with the "functionnal style" to write code.

The following code create two threads. One for a tcp server that accepts one client and sends him the "test" string. In the other thread, there is a client that reads in a socket after 2 seconds of wait.

open Sys
open Unix
open Lwt


let host = Unix.inet_addr_loopback (* 127.0.0.1 *)
let port = 6600
let max_pending_request = 10

let () = Lwt_log.add_rule "*" Lwt_log.Info

let send_message oc =
  Lwt_io.write_line oc "test"

let accept_connection conn =
  let fd, _ = conn in
    let oc = Lwt_io.of_fd Lwt_io.Output fd in
    Lwt.on_failure (send_message oc ) (fun e -> Lwt_log.ign_error (Printexc.to_string e));
    Lwt_log.info "New connection" >>= return

let create_server sock =
  Lwt_unix.setsockopt sock SO_REUSEADDR true;
  Lwt_unix.bind sock @@ ADDR_INET(host, port);
  Lwt_unix.listen sock max_pending_request;
  Lwt_unix.accept sock >>= fun conn ->
    accept_connection conn >>= fun () ->
      Lwt.return (Lwt_unix.shutdown sock SHUTDOWN_SEND)

let sock_recv sock maxlen =
  let str = Bytes.create maxlen in
  Lwt_unix.recv sock str 0 maxlen [] >>= fun recvlen ->
    return (String.sub str 0 recvlen)

let sock_read sock =
    Lwt_unix.sleep 2.0 >>= fun () ->
     ignore(Lwt_unix.connect sock @@ ADDR_INET(host, port));
      sock_recv sock 1024 >>= fun answer ->
        Lwt_io.write_line Lwt_io.stdout answer

let () =
  let create_socket () = Lwt_unix.socket PF_INET SOCK_STREAM 0 in
    let server_sock = create_socket () in
    let client_sock = create_socket () in
      let threads = Lwt.join [create_server server_sock;
                              sock_read client_sock] in
        Lwt_main.run threads

I use a lot of ";" and I have the feeling that it indicates that my code is not good (based on the code of others people I have read).

Is there anything in my code that I should avoid or some parts that could be improved?

\$\endgroup\$
  • \$\begingroup\$ Welcome to code review, I hope you get some good answers. \$\endgroup\$ – pacmaninbw Sep 19 '16 at 14:22
7
\$\begingroup\$

Using semicolons is OK. The IO is imperative and side-effectfull by default, so there is nothing to do here. By saying Lwt_unix.listen sock max_pending_request you're just setting a property inside the kernel. It is not blocking, so there is no need to create a thread here.

However, you should never use ignore function, to cast away the Lwt.t type. This is an error. The returned value is a thread, and the effect of connection will not happen, until the thread is determined. So, when you call sock_recv on the sock it might fail with ENOTCONN error. In case, when you really don't need the result of the computation, from a thread, and you don't need to sync with it, you can use Lwt.ignore_result or Lwt.async function, to create a background thread, that will be handled by the system. Note, however, that any exception thrown from the ignored computation will terminate the program, that basically means, that the the computation should never throw exceptions.

And few stylistic notes:

  1. Your indentation is incorrect. Use ocp-indent for indentation, it handles monadic operands (that shouldn't be nested).

  2. You can deconstruct values basically in any context, where you can bind, e.g.,

    let accept_connection conn =
      let fd, _ = conn in
    

    can be written as

    let accept_connection (fd,_) =
    
  3. Instead of using expr >>= return value pattern, you can use the map operator >|=. For example, the following

      Lwt_unix.recv sock str 0 maxlen [] >>= fun recvlen ->
      return (String.sub str 0 recvlen)
    

    Can be rewritten as,

      Lwt_unix.recv sock str 0 maxlen [] >|= String.sub str 0
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.