I'm excited to show I've ported my first function to Roc ;) · beginners

I've ported my first full function to Roc from TypeScript. Anyone want to code review?

It was a little tricky figuring out what List functions to use here but I got through it.

Luke Boswell (Jun 25 2024 at 23:01):

It looks like a faithful port of the Typescript version - using recursion to check a number of properties of a list.

Luke Boswell (Jun 25 2024 at 23:02):

You could also implement the same functionality using headingsMatch = \headingA, headingB -> headingA == headingB but I guess there's less opportunity to explore the syntax. :smiley:

Luke Boswell (Jun 25 2024 at 23:04):

I think the module syntax has changed recently, it would now be module [headingsMatch] to expose this function.

Luke Boswell (Jun 25 2024 at 23:10):

Another idea, sometimes I like to use variables to document things instead of comments. So you could also write it like;

headingsMatch = \headingA, headingB ->

    isDifferentLength = List.len headingA != List.len headingB
    isMatchingEmpty = List.len headingA == 0 || List.len headingB == 0
    isFirstItemNotMatching = List.first headingA != List.first headingB

    if isMatchingEmpty then
        Bool.true
    else if isDifferentLength || isFirstItemNotMatching then
        Bool.false
    else
        headingsMatch (List.dropFirst headingA 1) (List.dropFirst headingB 1)

Ashley Davis (Jun 26 2024 at 00:03):

Sam Mohr (Jun 26 2024 at 00:04):

The best thing about this impl is that I understand what you're doing because it's easily readable. It's good that you added the if statements at the top as "guard" statements to check for basic stuff first, like list length disparity, but since this is written recursively, those checks will run every time we recurse, so N times for a list with N items. Consider embedding a helper recursing function that avoids repeating those checks

Sam Mohr (Jun 26 2024 at 00:08):

headingsMatch = \headingA, headingB ->
    if List.len headingA != List.len headingB then
        # Different lengths.
        Bool.false
    else if List.len headingA == 0 && List.len headingB == 0 then
        # Matching empty lists.
        Bool.true
    else
        listsAreEqual = \listA, listB ->
            when (listA, listB) is
                ([], []) -> Bool.true
                ([first, ..], []) -> Bool.false
                ([], [second, ..]) -> Bool.false
                ([first, .. as restOfFirst], [second, .. as restOfSecond]) ->
                    if first == second then
                        listsAreEqual restOfFirst restOfSecond
                    else
                        Bool.false

        listsAreEqual headingA headingB

Luke Boswell (Jun 26 2024 at 00:11):

headingsMatch = \headingA, headingB ->
    when (headingA, headingB) is
        ([], []) -> Bool.true
        ([a, .. as restA], [b, .. as restB] if a == b -> headingsMatch restA restB
        _ ->  Bool.false

Kiryl Dziamura (Jun 26 2024 at 00:15):

Just wanted to post a pattern match version but you beat me to it :big_smile:
Yeah, it leverages a powerful tool. I think from an education perspective (I see it’s part of the book), it’s a perfect place to show both versions I think

Luke Boswell (Jun 26 2024 at 00:16):

Luke Boswell (Jun 26 2024 at 00:17):

Yeah, one of the best parts of roc is the pattern matching. I find that, paired with recursion, can make for some really nice implementations for algorithms.

Sam Mohr (Jun 26 2024 at 00:18):

Also, most importantly, in typescript there may be reason to use a list because it's running in a dynamic system, but because you're literally just comparing 2 lists of strings, the simplest and most efficient way to do this is headingsMatch = \headingsA, headingsB -> headingsA == headingsB

Sam Mohr (Jun 26 2024 at 00:18):

Luke Boswell (Jun 26 2024 at 00:19):

Also, while we are looking at it. Before we had tuples I would hav used a Tag. I think it also looks pretty nice.

headingsMatch = \headingA, headingB ->
    when T headingA headingB is
        T [] [] -> Bool.true
        T [a, .. as restA] [b, .. as restB] if a == b -> headingsMatch restA restB
        _ -> Bool.false

Luke Boswell (Jun 26 2024 at 00:20):

Yeah, I think these kinds of examples are a great way to learn FP concepts like recursion. Sam makes a great point that for this specific use-case you would just use structural equality.

pyrmont (Jun 26 2024 at 00:20):

I'm super-duper brand-new to Roc so I wanted to test my intuition but in this specific case would a naïve comparison be (probably) as efficient because the String comparison operator does optimisations like checking length for you? (Understand that this is an algorithm exercise and so the point is to be able to write the procedure from first principles as it were.)

pyrmont (Jun 26 2024 at 00:21):

Kiryl Dziamura (Jun 26 2024 at 00:24):

I like how this simple example with the sequence of refactorings tells so much about the language!

Sam Mohr (Jun 26 2024 at 00:25):

pyrmont (Jun 26 2024 at 00:26):

Luke Boswell (Jun 26 2024 at 00:28):

Sam Mohr (Jun 26 2024 at 00:29):

Yeah, I was gonna say... if it's only used to test itself and nowhere else, then probably

Sam Mohr (Jun 26 2024 at 00:29):

But if it's used only in testing, but it's a helper to test something else, then I'd say no warning

Sam Mohr (Jun 26 2024 at 00:30):

Kiryl Dziamura (Jun 26 2024 at 00:38):

Tests are dead code unless it's tests execution. Unless the def is exposed, it is expected to have the warning

Luke Boswell (Jun 26 2024 at 01:37):

Richard Feldman (Jun 26 2024 at 02:55):

it definitely needs to be possible to write test helper functions and constants that are only ever referenced in tests! Those should not be reported as unused; that would make them essentially impossible to use :big_smile:

Luke Boswell (Jun 26 2024 at 03:02):

@Richard Feldman should I reopen that issue? I'm not 100% becuase of the specific issue reported is about recursion. Maybe I've found an edge case from somehwere.

Kiryl Dziamura (Jun 26 2024 at 05:20):

I think test helpers would require explicit identification then. I don’t think it’s a good idea to lock dead code with tests

Richard Feldman (Jun 26 2024 at 05:49):

hm, as in you write something that’s not intended to be a test helper, but also isn’t exposed, but is referenced in a test, and therefore never gets reported as unused even though it’s actually dead code?

Richard Feldman (Jun 26 2024 at 05:50):

actually we could address that situation by reporting “dead tests” which don’t reference anything exposed (including indirectly)

Richard Feldman (Jun 26 2024 at 05:52):

which would also address the other scenario, because after deleting the dead tests, the not-actually-a-helper would no longer be referenced anywhere, and would be reported as unused

Sam Mohr (Jun 26 2024 at 05:52):

module [foo]

foo = \a ->
    a + 1

bar = \b ->
    b - 1

baz = \c ->
    c * 2

expect
    foo 3
    |> bar
    |> == 3

expect
    baz 4 == 8

foo is exported, so it's not dead code. bar isn't exported, but it's used to text something that was exported, so it's not dead code. baz was only used in tests by itself, so it's dead code

Sam Mohr (Jun 26 2024 at 05:52):

Reporting "dead tests" would be a simpler way to achieve that goal than to do analysis on whether a function was used in an expect with an exposed function.

Kiryl Dziamura (Jun 26 2024 at 10:34):

However, it would make test helpers easy to leak into release env. Probably not a problem but without distinguishing mechanism, there is no difference between test helpers and just helpers

Brendan Hansknecht (Jun 26 2024 at 15:12):

Yeah, if we wanted strong separation of tests from code, we would need something like test modules in rust

Kasper Møller Andersen (Jun 26 2024 at 16:29):

An aside on this: at work we've built a component library, where each component has a set of examples. We occasionally run into components having dead code which isn't caught because there's still an example which exercises it. But because it's not of use in production, we would rather remove it.

But we also have the reverse, where we have a library of utility functions we can use, and for that, we also keep dead functions around sometimes because they can still be useful when you need them.

So dead code detection and what to do about that code definitely can get more complicated than just "is it called anywhere?"

Kiryl Dziamura (Jun 26 2024 at 17:42):

Ashley Davis (Jun 26 2024 at 22:14):

Thanks everyone. There's a lot to process here. Pattern matching is very cool, but seems very different to the languages I am coming from. I'm going back to the Roc tutorial to try and understand it better.

Ashley Davis (Jun 26 2024 at 22:15):

What's the difference between the tag and tuple versions? I can read the tuple version and understand that, but the tag version is a form of syntax I haven't seen in Roc yet.

Brendan Hansknecht (Jun 27 2024 at 01:15):

Brendan Hansknecht (Jun 27 2024 at 01:17):

headingsMatch = \a, b ->
  when (a, b) is
    ([], []) -> Bool.true
    ([a, .. as restA], [b, .. as restB] if a == b -> headingsMatch restA restB
    _ -> Bool.false

Brendan Hansknecht (Jun 27 2024 at 01:18):

So really only the tuple version should be used/recommend for language consistency

Sam Mohr (Jun 27 2024 at 01:20):

They're identical, but I think people prefer the tag approach because it's slightly more terse

Brendan Hansknecht (Jun 27 2024 at 01:20):

The tag version just is constructioning a tag with the type [T Str Str]. It only has a single possible variant. Assuming heading is List Str

Sam Mohr (Jun 27 2024 at 01:20):

Sam Mohr (Jun 27 2024 at 01:21):

Brendan Hansknecht (Jun 27 2024 at 01:21):

Sam Mohr (Jun 27 2024 at 01:21):

Sam Mohr (Jun 27 2024 at 01:22):

Brendan Hansknecht (Jun 27 2024 at 01:22):