Package no longer solves the problems portrayed in the readme.

[Helam24]

I realize i'm years late to this but the readme is incorrect now that the package has been changed to be less safe with the changes to how it stores things and more so the support for initialization via literals.

Aside from the fact that "A compile-time guarantee that a collection contains a value." is no longer true, the readme also presents multiple issues in code that illustrate the usefulness of this package, but a few of them are no longer valid.

The example in the section "Better interface with 3rd party APIs" is no longer correct. It states that we can prevent the problematic code from ever happening in the first place by using non empty sets instead but this is no longer true. Now the problematic code is not only still possible but also causes a crash.

The example in the section "More expressive data structures" is also no longer correct. It shows that trying to initialize with an empty array gives a compiler error and while it does, it is only because the initializer that takes a collection is fail-able and returns an optional, not because initializing with an empty array generates a compiler error. If instead it was .invalid([]) or .invalid(.init()) it would compile just fine and then crash at runtime.

Again I know this is years late to the party but I was really excited to have a compile-time guarantee of non emptiness after watching the original video in the pointfree series about NonEmpty, I included it in my project and started using it, only to later discover that it is no longer safe.

I feel the risk is too high that eventually even someone who is intimately familiar with this package will be dealing with an optional NonEmpty collection and accidentally use ?? [] without thinking about it and cause a crash in production in a scenario that isn't usually tested.

I understand that using [] with NonEmpty is being considered a programmer error, but force unwrapping .first! on a regular array when that array is empty is also a programmer error, and I thought the purpose of this package was to make that impossible in the first place.

Also completely understand if I don't like it I don't have to use it or I can modify it or whatever but I still wanted to voice that I feel the move away from the safety seems counter to what I thought the original goal of the package was, and point out that the readme is misleading in the current state.

[mbrandonw]

Hi @Helam24, the readme should definitely be updated, but we do believe that the package solves the problems it originally set out to.

First, some history. The library started exactly in the way that was built in episodes, but after about 2 years of using it we realized there are some problems with it. It just wasn't ergonomic or performant at all. So, we ultimately decided to refactor it to be "technically" compile-time safe, though the internals are a bit unsafe: #26. Because we constrained how NonEmpty can be used with its public API, it was just as safe as before the PR.

This is a common trick to employ in the hopes of making something more ergonomic and performant. You often need to resort to unsafe APIs under the hood and present a safe API externally. And we're taking the compromise that if we try our hardest to make the external API safe, then we are ok with calling the library "safe". Otherwise any library using a single pointer under the hood cannot be thought of as "safe".

Further, the screenshots you pointed out are definitely very old, and should be updated. But we did make another concession to the ergonomics of NonEmpty by allowing it to conform to ExpressibleByArrayLiteral. This means you can technically do non-sensical things like this:

var xs: NonEmptyArray<Int> = []

Our view was that the ergonomic win of being able to use array syntax is greater than the safety concern, because array literal syntax is something very local, and hopefully it would be clear that is not valid to use an empty array.

However, the ExpressibleByArrayLiteral conformance is the only safety escape hatch in the library. So, before adding that the library was just as safe to use as it was when we modeled things as (head: A, tail: [A]).

We could re-consider the ExpressibleByArrayLiteral conformance. Maybe it wasn't the right choice after all. But, it's a very difficult problem to handle correctly. Currently it is our opinion that basically no one is going to want to use a NonEmpty type without that conformance. It's just too much of a pain. But we may be wrong!

I am curious: have you used a NonEmpty type in many places across a code base before? Have you not been annoyed by some of its ergonomics quirks?

[Helam24]

@mbrandonw thank you for your response. I acknowledge the difficulty of the task you're trying to solve, and my lack of experience with it.

I have not used it at scale before so I have not yet had the chance to run into the quirks you're talking about, I was just starting to add it across a project before I realized that initialization via literals was possible and could cause a crash and am now trying to decide if I want to redo the work and add back in all the empty checks or if I want to make a separate version of NonEmpty that does not support initialization via literals.

I agree that this is largely non-sensical and relatively easy to avoid:

var foo: NonEmptyArray<Int> = []

The scenarios that scare me involve nil coalescing to an empty array, which is something I believe to be fairly common to do for regular arrays. Imagine the following scenarios where .ingredients is a non empty array.

let ingredientNames = product?.ingredients.map(\.name) ?? []

or

checkAvailability(for: product?.ingredients ?? [])

The above scenarios feel less non-sensical to me. The words NonEmpty do not show up on the same line and may not even be in the same file. It looks and acts largely like a normal array, it seems reasonable that someone might not realize or remember what they're dealing with and mistakenly use ?? [] and introduce a crash waiting to happen. This would be difficult to catch in code reviews as well when viewing a diff that has no indicators that a non empty array is being used.

A compiler error in this case seems extremely desirable.

[mbrandonw]

Hi @Helam24, after discussing with @stephencelis we are more of the mind to sunset and archive this project rather than keeping it going in its current form. It's just a very difficult problem to solve, and we're not sure Swift is the right language for this tool.

Alternatively, I could see us releasing a 1.0 of the library that drops the literal conformance, and that would probably be the last release we ever do for the library.

[Helam24]

@mbrandonw well shoot, it certainly wasn't my intention to spur something like that. I had watched the original video and felt all motivated to try to use it all over the place. I imagined that you must use it in your projects regularly, is that not the case? If you have a bunch of experience with it and have found it not to be worth the hassle maybe I should "take your word for it" and stick to handling empty the good old fashioned way.

Regardless of what direction you go with this, I want to say thank you for your videos. I've learned a lot from them and feel I have improved as a developer because of them.

[mbrandonw]

Ha, it's no worries @Helam24. This is a good conversation to have! I do feel at the very least dropping the conformance is a good thing to do, and we can re-evaluate after that.

We have used the tool in a few key areas, but never in a very pervasive manner throughout a code base.