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 >= 0... [Read Full]
markdown guide
 

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 :)

 

+1

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

 

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 :)

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"
  ]

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!

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.

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

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

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.

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

 

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)
 

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

 

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 :)

code of conduct - report abuse