Request: show your most indented code!

1

Hello! We’d like to gather some of your code that are overly indented, and try to assess a few things about the syntax and semantics.

  • Feel free to strip the real variable names and all.
  • JSX doesn’t count, of course =). Unless you feel it’s pathologically ugly.
  • Must be real production code, aka no excessive code golfing.
  • This is asking for code that’s naturally written in an indented way, not asking for formatter bugs.
  • Feel free to message me if you don’t want to show the snippets in public.

Thanks in advance!

1 Like
2

Assuming you’re ok with a ReasonML example?

My record is 22 indents (24 if you don’t count the formatter keeping some of it on one line). This is a HTML model test, written in mocha. The test begins triple indented with test suite grouping, adds an extra local open indent for good measure, and creates a nested HTML table structure through model generation methods.

It’s using lists, but that’s only because we created our test framework before we realised arrays were better in JS. The framework is too widely used to migrate now and we figure the differences don’t matter as much in test code.

      it("should empty table cells that are deep in the tree if selection has nested tables", () => {
        testDeleteBackward(
          ~model=
            SlateMarkup.(
              toEditor([
                block(
                  "table",
                  [
                    block(
                      "tbody",
                      [
                        block(
                          "tr",
                          [
                            block(
                              "td",
                              [
                                block(
                                  "table",
                                  [
                                    block(
                                      "tbody",
                                      [
                                        block(
                                          "tr",
                                          [
                                            block("td", [text("ab")]),
                                            block("td", [text("cd")]),
                                          ],
                                        ),
                                      ],
                                    ),
                                  ],
                                ),
                              ],
                            ),
                            block("td", [text("cd")]),
                          ],
                        ),
                      ],
                    ),
                  ],
                ),
              ])
            ),
          ~expectedModel=
            SlateMarkup.(
              toEditor([
                block(
                  "table",
                  [
                    block(
                      "tbody",
                      [
                        block(
                          "tr",
                          [
                            block(
                              "td",
                              [
                                block(
                                  "table",
                                  [
                                    block(
                                      "tbody",
                                      [
                                        block(
                                          "tr",
                                          [
                                            block("td", [text("")]),
                                            block("td", [text("")]),
                                          ],
                                        ),
                                      ],
                                    ),
                                  ],
                                ),
                              ],
                            ),
                            block("td", [text("")]),
                          ],
                        ),
                      ],
                    ),
                  ],
                ),
              ])
            ),
          ~range=
            makeRange(
              ~anchorPath=[0, 0, 0, 0, 0, 0, 0, 0, 0],
              ~anchorOffset=1,
              ~focusPath=[0, 0, 0, 1, 0],
              ~focusOffset=1,
            ),
        )
      });
    });

Looking at it in isolation my first thought is perhaps it could be refactored to reduce the indentation, but that’s not a great idea in test code when the path references at the bottom must be specific to the model structure. In these sorts of tests using specific inputs can avoid tests passing incorrectly.

Setting aside test code, which is arguably an unrealistic example even if it is production code, my record is 13 indents. This happens in nested promise-based networking code; the function is nearly 200 lines long and it’s way too complex to try to extract a realistic sample. It probably can be refactored to reduce the indenting, but the whole file has been flagged for a rewrite to fix technical debt.

3

No, we are explicitly asking for ReScript code snippets.

Here is the converted version:

it(
  "should empty table cells that are deep in the tree if selection has nested tables",
  () => {
    open SlateMarkup
    testDeleteBackward(
      ~model=toEditor([
        block(
          "table",
          [
            block(
              "tbody",
              [
                block(
                  "tr",
                  [
                    block(
                      "td",
                      [
                        block(
                          "table",
                          [
                            block(
                              "tbody",
                              [
                                block(
                                  "tr",
                                  [
                                    block("td", [text("ab")]),
                                    block("td", [text("cd")]),
                                  ],
                                ),
                              ],
                            ),
                          ],
                        ),
                      ],
                    ),
                    block("td", [text("cd")]),
                  ],
                ),
              ],
            ),
          ],
        ),
      ]),
      ~expectedModel=toEditor([
        block(
          "table",
          [
            block(
              "tbody",
              [
                block(
                  "tr",
                  [
                    block(
                      "td",
                      [
                        block(
                          "table",
                          [
                            block(
                              "tbody",
                              [
                                block(
                                  "tr",
                                  [
                                    block("td", [text("")]),
                                    block("td", [text("")]),
                                  ],
                                ),
                              ],
                            ),
                          ],
                        ),
                      ],
                    ),
                    block("td", [text("")]),
                  ],
                ),
              ],
            ),
          ],
        ),
      ]),
      ~range=makeRange(
        ~anchorPath=[0, 0, 0, 0, 0, 0, 0, 0, 0],
        ~anchorOffset=1,
        ~focusPath=[0, 0, 0, 1, 0],
        ~focusOffset=1,
      ),
    )
  },
)

The test code is less interesting because it is essentially expressing tree data (kinda like JSX) — a case where right-drift is pretty common / unavoidable.

Would be great to see the mentioned 200 lines file because it seems to contain some relevant control flow / async code. If you can’t share it publicly, maybe send us a private message so we can get some clearer picture.

4

Right; the goal is to examine if there’s some category of use-cases that’s frequent enough to warrant some fixing. The HTML test is arguably a JSX-like thing where you’re modeling the tree, so it’s hard to say whether there’s anything to fix. Definitely not the easiest on the eyes but I’d say it represents the nature of the problem well, so maybe un-indenting it isn’t even a goal.

However one can argue that it’d be a nice feature if the block snippet was indented more like so:

block("table", [
  block("tbody", [
    block(...)
  ])
])

Which does make me think of whether we should adjust our brackets/braces hugging heuristics…

Anyway, an example of fixable bad indentation would be if there’s some use-case around decoding JSON fields and folks repeatedly do nested JSON option switch check. Or the famous server-side callback pyramid, etc.

5

Not sure if this counts, but unboxing an Option/Result value adds one level of indentation. And if you have many values to unbox, you have deep indentation because ReScript offers no letop-binding or something like this.

See https://github.com/nkrkv/jzon/blob/master/src/Jzon.res#L1276-L1352 for an extreme.

6

@ryyppy I’ll try to sanitise the other indented code and post it here. I have a busy week so it might take a day or two to find the time!

My team built a small result-based validation framework for cases like this. It composes multiple results together if they are all valid, and if there are any errors all errors are collected together. The map function type looks like this:

  let map4: (
    ~f: ('a, 'b, 'c, 'd) => 'e,
    vresult<'a, 'err>,
    vresult<'b, 'err>,
    vresult<'c, 'err>,
    vresult<'d, 'err>,
  ) => vresult<'e, 'err>

Using this to replicate your code structure, the 4 fields decoder would flatten to

Validation.map4(
  field1->Field.decode(fieldset)->Validation.fromResult,
  field2->Field.decode(fieldset)->Validation.fromResult,
  field3->Field.decode(fieldset)->Validation.fromResult,
  field4->Field.decode(fieldset)->Validation.fromResult,
  ~f=(val1, val2, val3, val4) => construct((val1, val2, val3, val4)),
)

(the added fromResult is because standard result errors need to be lifted into an array before they can be composed like this)

7

Nice. I was gonna write something basically like that.

Like @spyder showed, your indentation issue is from the lack of a proper helper function, not from the lack of letop.

Though we’re indeed looking for other such related snippets.

8

Sure. But isn’t it just moving the deep indentation to another place? I mean, how Validation.map15 would look like? I guess there will be 15-level nesting/indentation.

1 Like
9

Not with function composition. My switch statements are in functions and Validation.map composes them together. I can only show you the ReasonML infix version, since that’s what my project uses:

let map2 = (~f, r1, r2) => f <$> r1 <*> r2;
let map3 = (~f, r1, r2, r3) => f <$> r1 <*> r2 <*> r3;
let map4 = (~f, r1, r2, r3, r4) => f <$> r1 <*> r2 <*> r3 <*> r4;

But the non-infix version would be pretty similar, just with more brackets, and thanks to -> pipe formatting completely flat. Here’s the JS produced, to show you what the infix functions are doing.

function map4(f, r1, r2, r3, r4) {
  return apply(apply(apply(Belt_Result.map(r1, f), r2), r3), r4);
}

So in ReScript you’d have something like (I haven’t tested this):

let map4 = (r1, r2, r3, r4, f) =>
  r1
  ->map(f)
  ->apply(r2)
  ->apply(r3)
  ->apply(r4)
2 Likes
10

Dare I say that I prefer the pipe version much more! There might also be a much more descriptive for this situation.

Anyway let’s not derail this thread =) We still need more code examples.

Btw it’s possible that we’ve also got a generalized fix/printer idea for your block issue. Stay tuned.

2 Likes
11

I actually couldn’t find a horrible example. I think this is roughly the worst I have:

let reducer = (action, state) =>
  switch action {
  | Load(id) =>
    ReactUpdate.UpdateWithSideEffects(
      {...state, loading: true},
      self => {
        Future.map3(Trip.fetch(id), Element.fetchTripArray(id), Tag.fetchTripArray(id), (
          a,
          b,
          c,
        ) => (a, b, c))
        ->Future.tap(promises => self.send(LoadResponse(promises)))
        ->ignore
        None
      },
    )
  ...
12

Some nested structures in our codebase. Most of our code is JSX, but the nesting happens in the attributes, so they might as well be hoisted:

<TouchableHighlight
  style={Style.style(
    ~width=32.->Style.dp,
    ~height=32.->Style.dp,
    ~borderRadius=16.,
    ~overflow=#hidden,
    (),
  )}
  onPress={_ => {
    if value != "" {
      setValue(_ => "")
      let id = uuid()
      let _ = mutate(
        ~update=({writeQuery}, {data: mutateData}) => {
          lastEdgeIndex.current = 0
          let _ = writeQuery(
            ~query=module(MessageList),
            ~data={
              let edges = switch mutateData {
              | Some({createChatMessage: Some(message)}) =>
                let edge: MessageList.t_chatMessageList_edges = {
                  __typename: "ChatMessageEdge",
                  cursor: Some(message.id),
                  node: Some(message),
                }
                Belt.Array.concat([Some(edge)], edges.current)
              | _ => edges.current
              }
              {
                chatMessageList: Some({
                  ...chatMessageList,
                  edges: Some(edges),
                }),
              }
            },
            MessageList.makeVariables(~pixelRatio=PixelRatio.get(), ()),
          )
        },
        ~optimisticResponse=_ => {
          {
            createChatMessage: Some({
              __typename: "ChatMessage",
              id: id,
              text: Some(value),
              insertedAt: Some(Js.Date.make()),
              user: user,
              sent: false,
              pending: true,
            }),
          }
        },
        {message: value, id: id, pixelRatio: PixelRatio.get()},
      )->FFPromise.then_(_ => FFPromise.resolve())
    }
  }}>
  <Box flex=1. backgroundColor=colors.blue500 />
</TouchableHighlight>


<ReanimatedView
  style={
    open Style
    style(
      ~position=#absolute,
      ~height=dp(42.),
      ~left=dp(0.),
      ~transform=[
        translateX(
          ~translateX={
            open Reanimated
            StyleProp.float(
              Value.interpolateDynamic(
                animationState.React.current,
                {
                  inputRange: [Value.toAnimated(0.), Value.toAnimated(1.)],
                  outputRange: [
                    sub(Value.toAnimated(window.width), cancelWidth.React.current),
                    Value.toAnimated(window.width),
                  ],
                },
              ),
            )
          },
        ),
      ],
      (),
    )
  }
  onLayout={event => {
    let width = event.nativeEvent.layout.width
    if width > 0. {
      open Reanimated
      Value.set(cancelWidth.React.current, width->Value.toAnimated)
    }
  }}>

When building out objects using @obj it can lead to a pretty nested situation. This is still relatively ok (you can see that it can easily get more nested). It’s definitely more nested than JavaScript:

let id = "Chat"
let name = #F5_Chat
let screen = () => {
  let colors = Colors.getCurrentColors()

  open Navigation
  layoutChildren(
    ~component=layoutComponent(
      ~id,
      ~name,
      ~options=navigationOptions(
        ~layout=optionsLayout(
          ~componentBackgroundColor=colors.background,
          ~backgroundColor=colors.background,
          (),
        ),
        ~topBar=optionsTopBar(
          ~elevation=0.,
          ~drawBehind=true,
          ~noBorder=true,
          ~background=optionsTopBarBackground(~color="transparent", ()),
          ~backButton=optionsTopBarBackButton(~showTitle=false, ~color=colors.label, ()),
          (),
        ),
        (),
      ),
      (),
    ),
  )
}

let push = () => Navigation.pushChild(#TodayStack, screen())

All-in-all I didn’t really find very troubling deeply nested code in our codebase at all. So that’s a good thing!

13

Yes, thanks, that’s the kind I’m looking for.

14

One of the oldest parts of our codebase with some non-relevant code removed.

external useGoogleMaps: unit => ('a, 'b) = ""

module API = {
  external exec: (~coords: 'a) => Js.Promise.t<array<'b>> = ""
}

module Map = {
  @react.component
  let make = (~isActive, ~setSomething) => {
    let (map, autocomplete) = useGoogleMaps()

    React.useEffect3(() =>
      switch (map, autocomplete) {
      | (Some(map), Some(autocomplete)) =>
        let listener = autocomplete["addListener"]("place_changed", () => {
          let place = autocomplete["getPlace"]()

          switch (place["geometry"], place["types"]) {
          | (Some(geometry), Some(types))
            if Js.Array.includes("street_address", types) || Js.Array.includes("premise", types) =>
            switch geometry["location"] {
            | Some(location) =>
              map["setCenter"](location)
              map["setZoom"](20)

              let coords = location["toJSON"]()

              if isActive {
                API.exec(~coords)
                |> Js.Promise.then_(result =>
                  switch result->Belt.Array.get(0) {
                  | None => Js.Promise.resolve()
                  | Some(x) =>
                    setSomething(_ => Some(x))

                    Js.Promise.resolve()
                  }
                )
                |> ignore
              }
            | None => ()
            }
          | (Some(geometry), _) =>
            switch geometry["viewport"] {
            | Some(viewport) => map["fitBounds"](viewport)
            | None => ()
            }
          | _ => ()
          }
        })

        Some(() => listener["remove"]())
      | _ => None
      }
    , (map, autocomplete, isActive))
  }
}

Biggest issue here is usage of objects for google maps binding.

If place has optional geometry and geometry has optional location I can not write flat pattern matching

switch geometry {
 | Some({"location": Some({"place": place})) => ...
 | _ => ()
}
15

Here is mine:

open Slate

let withPlugins = (editor: Editor.t) => {
  let {deleteBackward} = editor
  let {isBlockStart, getClosestAncestorBlockEntry, isSelectionInside, getRootElementEntry} = module(
    Node
  )
  let {asLocation} = module(Path)
  let {removeNodesWithOptions} = module(Transforms)
  let {previous} = module(Editor)

  editor.deleteBackward = (. deletable) => {
    let delete = ref(true)

    switch Null.toOption(editor.selection) {
    | None => ()
    | Some(selection) =>
      let closestAncestorBlockEntry = editor->getClosestAncestorBlockEntry(selection.anchor.path)

      switch closestAncestorBlockEntry {
      | None => ()
      | Some((_, closestAncestorBlockPath)) =>
        // Anchor is at the beginning of the block
        if editor->isBlockStart(selection.anchor, closestAncestorBlockPath) {
          // #figcaption
          if editor->isSelectionInside(#figcaption) {
            // Do nothing
            delete := false
          }

          let rootElementEntry = editor->getRootElementEntry(selection.anchor.path)

          switch rootElementEntry {
          | None => ()
          | Some((_, rootElementPath)) =>
            let previousOptions = Editor.makePreviousOptions(~at=rootElementPath->asLocation, ())
            let previousRootNodeEntry = editor->previous(previousOptions)

            switch previousRootNodeEntry {
            | Some((previousRootNode, previousRootNodePath)) =>
              // Root element following #figure
              if Node.view(previousRootNode).kind === Some(#figure) {
                editor->removeNodesWithOptions(
                  Transforms.makeRemoveNodesOptions(~at=previousRootNodePath->asLocation, ()),
                )
                delete := false
              }
            | None => ()
            }
          }
        }
      }
    }

    if delete.contents {
      deleteBackward(. deletable)
    }
  }

  editor
}

I’m using a mutable ref variable because there is no explicit return.