# Request for code critique/reivew for first time learner coming from Rust

**URL:** <https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165>\
**Category:** Learning\
**Tags:** beginner\
**Created:** [May 24, 2026, 4:02am UTC](https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165 "2026-05-24T04:02:30Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![allonsyeet](https://avatars.discourse-cdn.com/v4/letter/a/c6cbf5/32.png) [@allonsyeet](https://discuss.ocaml.org/u/allonsyeet)\
**Post date:** [May 24, 2026, 4:02am UTC](https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165/1 "2026-05-24T04:02:31Z")

</div>

I decided that I’d learn OCaml by doing something basic I’m familiar with, so I wrote this file for solving a mass on a spring, and I wanted to ask if people could provide any critique/suggestions for improvement/style changes as this is literally the first program I’ve written.

I’m using a module because I want to get familiar with them, so everything related to a simulation is in this module. I also defined a signature for a simulation type but I’m not sure how to use it because I’d need like 5 `M:S with type` kind of statements.

```ocaml
module Simulation = struct
  (* Simulation interface. idk how to use it tho. ideas?*)
  module type S = sig
    type err
    type state
    type params
    type history
    type algorithms

    val step : state -> params -> algorithms -> ( state, err ) result
    val step_append : state -> params -> algorithms -> history -> ( state, err ) result
    val init_history : int -> history
  end

```

This submodule implements `Simulation.S` (though if I do that the associated types become, idk is opaque the right word? so I can’t use it from outside the module). The state of the simulation at any point is a position and velocity, I have some parameters for the simulation, a array storing the past states, and a list of algorithms that can be used to step.

```ocaml
  (* SHO simulation impl *)
  module SHO = struct
    type err = string
    type state = {
      x : float;
      v : float;
    }
    type params = {
      k: float;
      dt: float;
    }
    type history = {
      ptr: int ref;
      xs: float array;
      vs: float array;
      ts: float array;
    }

    type algorithms =
      | Euler
      | Leapfrog
      | RK2
      | RK4

```

I’m not sure if these functions are idiomatic ocaml. So I’d appreciate any feedback.

```ocaml
    let step ( s:state ) ( p:params ) ( m:algorithms ) = 
      match m with
        | Euler -> 
            Ok {
              x = s.x +. s.v *. p.dt;
              v = s.v +. (-. p.k *. s.x) *. p.dt;
            }
        | _ -> Error "unimplemented!" 

    let init_history n = {
      ptr = ref 0;
      xs = Array.create_float n;
      vs = Array.create_float n;
      ts = Array.create_float n;
    }

    let push_state (p:params) (h:history) (s:state) =
      let n = h.ptr.contents + 1 in
      if n >= Array.length h.ts then Error "no space left"
      else Ok (  
        h.ts.(n) <- (h.ts.(n-1) +. p.dt);
        h.xs.(n) <- s.x;
        h.vs.(n) <- s.v;
      )

    let step_append (s:state) (p:params) (m:algorithms) (h:history) =
      let s' = step s p m in
      if Result.is_error s' then s'
      else let s'' = Result.get_ok s' in
      match push_state p h s'' with
      | Ok () -> Ok s''
      | Error msg -> Error msg
  end
end

```

I suppose this block just depends on how the above code was written, but once again, any suggestions are welcome. Maybe better ways to do string formatting or printing of records?

```ocaml
let () =
  Simulation.SHO.(
    let h = init_history 10 in
    let rec loop s p m n =
      if n <= 0 then () else
      match ( step_append s p m h ) with
      | Ok s' -> (
          print_string "v = "; print_float s'.v; print_endline "";
          print_string "x = "; print_float s'.x; print_endline "";
          loop s' p m (n-1)
        )
      | Error msg -> print_string "error: "; print_endline msg
    in
    loop {x = 1.0; v = 0.0} {k = 1.0; dt = 0.001} Euler 15
  )

```

tysm for taking the time to read/comment \<3

> **Copy-pastable code in one chunk**
>
> ```ocaml
> module Simulation = struct
> (* Simulation interface. idk how to use it tho. ideas?*)
> module type S = sig
> type err
> type state
> type params
> type history
> type algorithms
> 
> val step : state -> params -> algorithms -> ( state, err ) result
> val step_append : state -> params -> algorithms -> history -> ( state, err ) result
> val init_history : int -> history
> end
> 
> (* SHO simulation impl *)
> module SHO = struct
> type err = string
> type state = {
> x : float;
> v : float;
> }
> type params = {
> k: float;
> dt: float;
> }
> type history = {
> ptr: int ref;
> xs: float array;
> vs: float array;
> ts: float array;
> }
> 
> type algorithms =
> | Euler
> | Leapfrog
> | RK2
> | RK4
> 
> let step ( s:state ) ( p:params ) ( m:algorithms ) = 
> match m with
> | Euler -> 
> Ok {
> x = s.x +. s.v *. p.dt;
> v = s.v +. (-. p.k *. s.x) *. p.dt;
> }
> | _ -> Error "unimplemented!" 
> 
> let init_history n = {
> ptr = ref 0;
> xs = Array.create_float n;
> vs = Array.create_float n;
> ts = Array.create_float n;
> }
> 
> let push_state (p:params) (h:history) (s:state) =
> let n = h.ptr.contents + 1 in
> if n >= Array.length h.ts then Error "no space left"
> else Ok (  
> h.ts.(n) <- (h.ts.(n-1) +. p.dt);
> h.xs.(n) <- s.x;
> h.vs.(n) <- s.v;
> )
> 
> let step_append (s:state) (p:params) (m:algorithms) (h:history) =
> let s' = step s p m in
> if Result.is_error s' then s'
> else let s'' = Result.get_ok s' in
> match push_state p h s'' with
> | Ok () -> Ok s''
> | Error msg -> Error msg
> end
> end
> 
> let () =
> Simulation.SHO.(
> let h = init_history 10 in
> let rec loop s p m n =
> if n <= 0 then () else
> match ( step_append s p m h ) with
> | Ok s' -> (
> print_string "v = "; print_float s'.v; print_endline "";
> print_string "x = "; print_float s'.x; print_endline "";
> loop s' p m (n-1)
> )
> | Error msg -> print_string "error: "; print_endline msg
> in
> loop {x = 1.0; v = 0.0} {k = 1.0; dt = 0.001} Euler 15
> )
> 
> ```

---

<div class="post-metadata">

**Author:** ![nojb](https://sea2.discourse-cdn.com/flex020/user_avatar/discuss.ocaml.org/nojb/32/519_2.png) [@nojb](https://discuss.ocaml.org/u/nojb)\
**Post date:** [May 24, 2026, 5:42am UTC](https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165/2 "2026-05-24T05:42:58Z")

</div>

Hello @allonsyeet, welcome to the forum!

Your code looks quite good at a glance. A few suggestions/remarks (opinions below, caveat emptor).

> [@allonsyeet](#):
>
> I’m using a module because I want to get familiar with them,

One of the purposes of modules is to serve as a structuring device for large-scale programs, and they can be awkward to use in small programs. My suggestion to beginners it always to concentrate on the bread and butter of OCaml: functions, datatypes, pattern matching. That subset of OCaml is simple and powerful, and can get you surprisingly far. You can broach modules once you have a clear need for them 🙂

> [@allonsyeet](#):
>
> I’m not sure if these functions are idiomatic ocaml. So I’d appreciate any feedback.

A few comments:

- The field `ptr` can be declared as `mutable` instead of using a `int ref`. The difference is subtle but important semantically: an `int ref` introduces an indirection; it is a value that exists on its own and can therefore be shared by different parts of the code, which you probably want to avoid as it means more chances of “action at a distance”. Using a `mutable` field instead allows mutability without introducing the extra indirection, which is both more efficient and reduces the possible set of side-effects in your code.

- You annotate function parameters with their types. There is nothing wrong about this, but with experience OCaml programmers typically tend to elide the type annotations (unless they are needed for disambiguation purposes). The reason is that it makes it simpler to refactor the code: no need to update the annotations when the type of arguments changes.

- In `push_state` you have `Ok (e1; e2; e3)`. There is nothing wrong with this, but one would typically write `begin e1; e2; e3; Ok () end` which more clearly shows the structure of the code.

- In `step_append` you switch cases using `Result.is_error`, `Result.get_ok`, etc. Instead you should use pattern matching

Incidentally, it is refreshing to see people still try to properly learn a language instead of oursourcing understanding to an AI.

Cheers,  
Nicolas

---

<div class="post-metadata">

**Author:** ![allonsyeet](https://avatars.discourse-cdn.com/v4/letter/a/c6cbf5/32.png) [@allonsyeet](https://discuss.ocaml.org/u/allonsyeet)\
**Post date:** [May 24, 2026, 9:32am UTC](https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165/3 "2026-05-24T09:32:18Z")

</div>

Hiya! tysm for the deailed response!

> [@nojb](#):
>
> The field `ptr` can be declared as `mutable` instead of using a `int ref`. The difference is subtle but important semantically: an `int ref` introduces an indirection; it is a value that exists on its own and can therefore be shared by different parts of the code, which you probably want to avoid as it means more chances of “action at a distance”. Using a `mutable` field instead allows mutability without introducing the extra indirection, which is both more efficient and reduces the possible set of side-effects in your code.

So I’ve updated the `type history` to this

```ocaml
type history = {
  mutable ptr: int;
  xs: float array;
  vs: float array;
  ts: float array;
}

```

> [@nojb](#):
>
> You annotate function parameters with their types. There is nothing wrong about this, but with experience OCaml programmers typically tend to elide the type annotations (unless they are needed for disambiguation purposes). The reason is that it makes it simpler to refactor the code: no need to update the annotations when the type of arguments changes.

So my reasoning for that is otherwise the function becomes harder to read without something like an LSP that can tell you the type of the function because the variable names are just single letters, though I suppose that can be fixed with better naming or a comment nearby.

> [@nojb](#):
>
> In `push_state` you have `Ok (e1; e2; e3)`. There is nothing wrong with this, but one would typically write `begin e1; e2; e3; Ok () end` which more clearly shows the structure of the code.

That makes much more sense! I did not know about the `begin ... end` syntax.

> [@nojb](#):
>
> In `step_append` you switch cases using `Result.is_error`, `Result.get_ok`, etc. Instead you should use pattern matching
> 
> ```auto
> match s' with
> | Error _ as err -> err
> | Ok s'' -> ...
> 
> ```
> 
> Using pattern matching provides the compiler with more information about the intent of your code, and the compiler can help you in return: warn you if you have not handled all possible cases, if you have useless overlapping cases, etc. In short: pattern matching should be the first tool you reach for in most circumstances.

I rewrote the function like so (haven’t removed the type annotations yet)

```ocaml
let step_append (s:state) (p:params) (m:algorithms) (h:history) =
  let s' = step s p m in
  match s' with
  | Ok s'' -> (
    match push_state p h s'' with
    | Ok () -> Ok s''
    | Error _ as err -> err
  )
  | Error _ as err -> err

```

Since the idiom of `if Ok do X else return error` keeps repeating, I was wondering does OCaml have syntax for early returning in case of an error variant like Zig’s `try` or Rust’s `?` operator? I found out about [binding operators](https://ocaml.org/manual/5.2/bindingops.html) and I know the corresponding operators are in `Result.Syntax` but I’m not completely sure how to use them yet. I don’t want to open the module globally and I don’t know how to do it in a function scope.

Once again, thank you for the detailed response, really appreciate it 🙂 .

Edit: Okay just learned about the `let open ... in` syntax so I came up with this.

```ocaml
let step_append (s:state) (p:params) (m:algorithms) (h:history) =
  let open Result.Syntax in
  let* s' = step s p m in
  match push_state p h s' with
  | Ok () -> Ok s'
  | Error _ as err -> err

```

Looked up the definition of `let*` operator and it’s the `Result.bind` operation which is:

```ocaml
let bind r f = match r with Ok v -> f v | Error _ as e -> e

```

Which is exactly the early return if I’m not wrong? The let\* statement says take the result from `step s p m`, and if it’s `Ok`, apply the function `fun s' -> match push_state p h s' with Ok () -> Ok s' | Error _ as err -> err` to the inner state. As far as I understand.

Edit 2: I realized that `step_append` can be simplified even further since we have the binding operators

```ocaml
let step_append s p m h =
  let open Result.Syntax in
  let* s' = step s p m in
  let* _ = push_state p h s' in
  Ok s'

```

---

<div class="post-metadata">

**Author:** ![nojb](https://sea2.discourse-cdn.com/flex020/user_avatar/discuss.ocaml.org/nojb/32/519_2.png) [@nojb](https://discuss.ocaml.org/u/nojb)\
**Post date:** [May 24, 2026, 11:26am UTC](https://discuss.ocaml.org/t/request-for-code-critique-reivew-for-first-time-learner-coming-from-rust/18165/4 "2026-05-24T11:26:20Z")

</div>

> [@allonsyeet](#):
>
> That makes much more sense! I did not know about the `begin ... end` syntax.

`begin ... end` is equivalent to parentheses, but is often used to delimit expression sequences (which are executed for side-effects).

> [@allonsyeet](#):
>
> Edit: Okay just learned about the `let open ... in` syntax so I came up with this.

Yep, that’s it!

Cheers,  
Nicolas