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?

pacmaninbw
26.2k13 gold badges47 silver badges113 bronze badges
asked Sep 19, 2016 at 14:16
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to code review, I hope you get some good answers. \$\endgroup\$ Commented Sep 19, 2016 at 14:22

1 Answer 1

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
    
answered Sep 19, 2016 at 22:01
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.