DEV Community

Can you spot the problem in the implementation of nthElement?

Dwayne Crooks on August 04, 2019

There is a problem with this implementation of nthElement: nthElement : List a -> Int -> Result String a nthElement list n = if n >= ...

Read full post

leojpod • Aug 4 '19

Just by a quick glance I'd say that your pluralise method isn't fed the proper number.
It should be (n+1) to get the remaning length I guess.

I'll have another look at it later :)

Dwayne Crooks • Aug 5 '19

That's partially correct. There's one more thing you missed.

leojpod • Aug 5 '19

it lacks a space here too

... String.fromInt (n+1) ++ pluralize n ...

here you'll get something like 1elements instead of 1 elements

to be honest I'd rather use something like elm-string-interpolate to avoid having to think of these string concatenations :)

Dwayne Crooks • Aug 5 '19

it lacks a space here too

precisely

to be honest I'd rather use something like elm-string-interpolate to avoid having to think of these string concatenations

I won't add a dependency just for that. But, the way I wrote it did lead to a subtle bug so maybe doing it in the following way would have been clearer:

String.join " "
  [ "List too short by"
  , String.fromInt (n+1)
  , pluralize (n+1) "element" "elements"
  ]

leojpod • Aug 5 '19

I would add a deps for this :D it's just in my test-deps, besides it might be useful in the rest of your app. I mean in the front-end we often have to show text with some values that we fetch from different places: using an utility or making your own to get that type of code:

"List too short by {} element{}" |> interpolate [ String.fromInt (n+1), pluralize (n+1) "" "s"]

furthermore, I'd actually make a interpolate function that takes a list of a custom type:

1st define a type like this:

type InterpolationValue a
    = IntValue Int
    | StringValue String
    | CurrencyValue Float 
    | ... 
    | CustomValue (a -> String) a

this way my interpolate function would look like this:

interpolate: List InterpolationValue -> String -> String

then I think my code would be much easier to read and also potentially much easier to i18nize it later on :)

But that's again just a matter of personal taste!

Dwayne Crooks • Aug 5 '19

To be clear, I really meant that in this project that I'm working on I won't add a dependency for it. Since this particular function, nthElement, is quite insignificant.

However, I do agree that if I'm working on a project where I am doing a lot of this sort of stuff then abstracting the task will have its benefits.

This discussion reminds me of Formatting in Haskell. Which I first came across in "Related Work" in url's README.

leojpod • Aug 5 '19

Sorry I didn't mean to come across aggressive or arogant :)

the formatting article is an interesting read! thanks for the link.

Dwayne Crooks • Aug 7 '19

Sorry I didn't mean to come across aggressive or arogant

I didn't think so at all. But, no worries. Thanks for sharing your thoughts. I appreciated it.

leojpod • Aug 7 '19

Cheers mate.
Keep up writing, your posts are cool!

leojpod • Aug 4 '19

you could perhaps optimize it to leverage the "tail-recurssive" optimization by doing something like this btw:

nthElementHelper : List a -> Int -> Result String a
nthElementHelper list n =
  case (n, list) of
    (_, []) ->
      Err ("List too short by " ++ String.fromInt (n+1) ++ pluralize n "element" "elements")

    (0, x :: _) ->
      Ok x
    (nth, _ :: rest) ->
      nthElementHelper rest (nth-1)

leojpod • Aug 4 '19

although come to think of it you're already doing "tail-recurssion"

Dwayne Crooks • Aug 5 '19

yeah, I'm already doing that

leojpod • Aug 5 '19

I still prefer the case of on a tuple than a case of with some if then else branching inside but that's personal taste :)