https://kotlinlang.org logo
Join Slack
Powered by
Quick question for all of you: as a functional-ori...
# arrow
o
Quick question for all of you: as a functional-oriented programmer, I love to use the NonEmpty... variants of Lists and Sets much more then the "normal" ones, whenever it is a business requirement that there is at least one entry. So, I frequently revisit my code and change signatures to be a 
NonEmptyList
rather than a 
List
. A byproduct that I do not like that much is that when these elements are used in user output in simple 
println
or logs, the 
toString()
output changes from
Copy code
List -> [foo, bar]
NonEmptyList -> NonEmptyList(foo, bar)
This is something I wish I could prevent, because more than once I needed to change the logging code to explicitly state 
"${stuff.joinToString()}"
instead of just 
"$stuff"
. Is it really good and necessary that Arrow's non-empty variants print their own name in their 
toString()
implementations?
🤔 1
r
I ran into this same issue. I think 
NonEmptyList
being considered a more restricted version of 
List
and also implementing the List interface should have the same 
toString
as 
List
. Same for all 
NonEmpty
variants with their respective super types.
❤️ 1
s
I'm open to this change, doesn't seem breaking ... ? 🤔 😅
➕ 1
t
p
I queried this recently and would be happy to raise a PR to remove the custom toString 😉
❤️ 1
I do wonder though if it should enforce the standard toString representation a la AbstractCollection rather than delegate to the wrapped collection - if you wrapped a collection with a custom toString you wouldn't expect 
NonEmptyList(myCustomCollection).toString()
to result in 
"MyCustomCollection(a, b c)"
o
If anybody has written code that depends specifically on the string produced by 
toString()
I would consider this extremely dangerous and wrong. So, we should be fine to remove custom 
toString()
methods for all non empty implementations.
p
☝️ This is exactly the case a colleague hit in a unit test 😉
🤣 2
o
Yes, it should enforce the use of the standard method and not simply delegate to the underlying implementation.
Theoretically, yes, some users might have written unit tests that have been adopted in their expectation of the current output. But that is easy to fix.
p
next arrow release is 2.2.x - feels "large" enough to me to make a "breaking" change to string representation that no one should rely on ...
👍 1
a
it seems that there are quite some caveats with the current implementation. The question is whether we should directly use the 
toString
from the underlying list, or we should do something else
➕ 1
o
IMHO we should make sure it outputs like a standard List or Set. Which means we should not use the underlying collection unless we are in control.
a
there is no "standard List"
👍 1
I mean, 
List
is just an interface, every type may override their 
toString
as they see fit
p
Currently looking at this now. There's the standard formatting from 
AbstractCollection
(effectively 
joinToString(", ", "[", "]") { if (it === this) "(this Collection)" else it.toString() }
) but cares needs to be taken to handle 
this
identity within the value class
o
From a user's perspective, I would simply but my request like this: make sure that the output of 
toString()
of 
nonEmptyList(...)
matches that of 
listOf(...)
. I do not expect 
toString()
to always state the exact type like currently. That is for all collections. For a data class, of course, the own type is relevant.
a
the problem is that there are two ways to get the behavior you expect, @Olaf Gottschalk : • we can specifically override 
toString
in 
NonEmptyCollection
• we can reuse the underlying's value 
toString
I lean towards the second choice, since it means 
NotEmpty
"invisible", but if you decidee to wrap another type you still get that information
p
my only concern with the second choice is seeing 
"MyCustomList(a, b c)"
as the 
.toString()
of something that isn't 
MyCustomList
- but the likelihood if that is slim given most collections implement the "standard" (but non-contractual) 
toString()
representation
a
o
I know, @Alejandro Serrano.Mena... But there is a "standard expectation" when coming from Kotlin and using lists and sets and putting them into string templates...
t
No test changes for this PR? 😄
a
yeah, maybe I should include some...
t
(Sorry 😛 im at work and its literally part of my job to point those things out to teams)
👮 1
a
p
given most/all usage of NonEmptyList/Set are via 
toNonEmptyList/Set
functions, they create and wrap a copy which will almost certainly have the "conventional" toString representation. Only those using 
wrapAsNonEmptyList/Set
functions would potentially expose an underlying non-conventional toString - and to wrap you need to opt in to 
@PotentiallyUnsafeNonEmptyOperation
which should be enough of a deterrent.
➕ 1
👍 1
o
Thanks for bringing this change in Arrow 2.2.0! Much appreciated!
arrow intensifies 4
4 Views
Open in Slack
PreviousNext
Powered by