dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

Experiencing Immutability Whiplash #890

Open leecommamichael opened 4 years ago

leecommamichael commented 4 years ago

Dart is not Flutter, but the process of writing our large Flutter app at work has revealed some pain points. When expressing the fundamentals of our app, Dart created friction that was avoidable.

Suppose this contrived, but typical scenario: We're going to sell candles in our app, the backend serves an inventory of candles. The Dart app will decode the data and populate the UI using Flutter.

@immutable
class Candle {
    Candle({@required this.scent});
    final String scent;

    @override int hashCode => (hashBuilder()
        ..hash(scent)).build();
    @override bool operator ==(other: dynamic) => other is Candle 
        && other.scent == scent;
    factory Candle.fromData(Map<String, dynamic>  decodedCandle) {
        return Candle(
            scent: decodedCandle['scent'] as String
        );
    }
}
class CandleList extends StatelessWidget {
    @override Widget build(BuildContext context) {
        final candleProvider = CandleProvider.of(context);
        final candles = candleProvider.getCandles();
        // For brevity all that was asynchronous is now synchronous and successful.
        return ListView.builder(itemCount: candles.length,
            itemBuilder: (context, index) => {
                final isTopSeller = candles[index] == candleProvider.topSeller;
                CandleRowItem(candle: candles[index], useFlashyBanner: isTopSeller);
            },
        );
    }
}

Everything works great! We used our hard-earned experience (and imaginary hashing package) to know that we couldn't tell if something was the "top seller" unless we implemented both == and hashCode (even though there was no lint that indicated otherwise while editing.)

But our requirements have been refined, the customers want to be able to buy different sizes of candles. Not all candles have the same sizes.

We will add the following field to our Candle class: final List<String> sizesAvailable;. This will require a change to each method in our class, while also breaking our CandleList widget even though it doesn't read that property!. It will also make writing tests less fun.

Our tests will need to perform deep comparison of the elements of lists of candles returned from the service, to avoid failed comparison of List hashCodes. Our Widget will fail to show a flashy banner for the top-selling product. Our Candle will no longer be comparable, as the non-deterministic hash of the list will poison the well.

Adding complexity to this use-case, this code is usually handleable through code generation and packages like Equatable. As a developer, I don't enjoy the latency to interaction a code-generator enforces, nor the need to remember that it exists and can write bugs.

If we look into how our CandleProvider works, it raises some more questions from my perspective.

class CandleProvider extends InheritedWidget {
    List<Candle> getCandles()  {
        final candlesResponse = GET "endpoint/v1/candles";
        final decodedResponse = jsonDecode(candlesResponse.body) as List;
        return decodedResponse
            .cast<Map<String, dynamic>>()
            .map((candleData) => Candle.fromData(candleData))
            .toList();
    }
}

When we use List.cast, our list is represented as a CastList. I'm assuming it was not feasible for all use-cases to typecheck elements per list operation, so we have this sort of intermediate representation to the list that we actually care about as end users. I'd rather not have to think about the fact that Dart would let this be a temporarily heterogeneous collection, and then throw if it pulls something out that it doesn't recognize.

I can't speak for everyone, but I think of Lists as values whose equality/identity is determined by the order and identity of those elements in the List. So the concept of needing a CastList in much of my code is foreign. As such, I didn't enjoy learning about List.cast and how/when to use it properly.

This is when we found the BuiltCollections and BuiltValue packages. I think these are awesome packages, but is it best for these to remain packages and not part of the language? Most, if not all, of the pain points I've raised so far were avoidable by embracing immutability. If we can't assume everyone wants to do that, can we offer an alternative way of modeling our data that is separate of the mostly mutable class ecosystem? I loved writing Swift structs for this reason, and I believe Kotlin has a similar concept with dataclasses.

The immutable collections and structures in Swift make it a joy to write (but waiting for the compiler does not), the solutions often lean into one another as well! If dart:convert worked with built_collections I wouldn't need List.cast<T> because the list can't change.

Dart is usually natural to write, and I wouldn't have created this issue if I didn't believe in it and desire for it to be better. If we want Dart to be held in the same regard as the other modern languages, I believe these are the utmost problems to be solved from the end-developer perspective. I fully understand that you all have been busy transforming Dart into a soundly-typed language with native compilation targets, but I hope those are the brunt of the backend distractions from the current user experience.

I think Dart could lean into this fresh-landscape by providing unique features like a nifty syntax to update immutable data. The conventional Instance.copyWith falls on it's face because it can't assign null. I'm not asking for a spread operator, but I'd be elated if we could express a copyWith that can assign null. I understand this is doable with Optionals which might be on the way?

P.S. I like Tuples, they'd play so well with the Selector widget from Provider...

Anyway, that was my Christmas list, sorry it was late. I just wanted to throw my 2 cents in and see if anyone else feels the same way. Thank you for building such a fun language to work with, the tooling is starting to get really good 😁 I appreciate the focus on developers.

lrhn commented 4 years ago

About the List.cast, it's not really necessary. You need to cast from dynamic to Map<String, dynamic> at some point because the JSON decoder returns a List<dynamic>. Possible approaches include;

return decodedResponse
    .map((candleData) => Candle.fromData(candleData as Map<String, dynamic>))
    .toList();

(where the cast can be implicit if you don't disallow implicit downcasts), or

return [for (Map<String, dynamic> map in decodedResponse) Candle.fromData(map)];

Using cast is not necessary because you don't want to do cast per element (you still do, that's what the wrapper does for you), but because it's the cheapest way to get a list where the run-time type satisfies your requirements. The actual list you have has a run-time type of List<dynamic>, You cannot get the Map<String, dynamic> elements out of that without a cast somewhere, but there are quite a lot of options regarding where you put it.

I generally never use the cast, but it was very useful during the migration to the Dart 2 "strong" type system where you could previously get away with using a List<dynamic> as a List<String>. These days, the collection-if and collection-for makes a lot of computations simpler.

lrhn commented 4 years ago

About copyWith ... I feel your pain. It is annoying that you cannot easily distinguish a nullable parameter with explict value null from an absent parameter. It's possible to get around that for implementable types, but not for unimplementable core types like int and String.

Workaround include:

There are no current plans for a platform SDK defined Option type. The work we do for Null Safety makes some uses of non-nullable types easier to work with, and easier to safely use null as no-value marker for, but nullable types are no different than today.

leecommamichael commented 4 years ago
return [for (Map<String, dynamic> map in decodedResponse) Candle.fromData(map)];

Using cast is not necessary because you don't want to do cast per element (you still do, that's what the wrapper does for you), but because it's the cheapest way to get a list where the run-time type satisfies your requirements. The actual list you have has a run-time type of List<dynamic>, You cannot get the Map<String, dynamic> elements out of that without a cast somewhere, but there are quite a lot of options regarding where you put it.

I generally never use the cast, but it was very useful during the migration to the Dart 2 "strong" type system where you could previously get away with using a List<dynamic> as a List<String>. These days, the collection-if and collection-for makes a lot of computations simpler.

D'oh! I guess that's a bit obvious isn't it? Thanks! When I was learning https://dart.dev/guides/language/sound-problems#invalid-casts was the top hit for this kind of problem, which advertises List.cast. It'd be nice to see this sort of thing documented there. Is that page accepting PR's from external authors?

There are no current plans for a platform SDK defined Option type. The work we do for Null Safety makes some uses of non-nullable types easier to work with, and easier to safely use null as no-value marker for, but nullable types are no different than today.

Interesting. We probably can't have Maybe or Option because the work would need to be done for some kind of ADT or breaking changes to enum and non-existent pattern matching? I feel like those are really useful additions, but Dart seems to still be in that kind of middleground of transitioning from "Better Javascript" to "Better Java". That's my own interpretation though :)

mnordine commented 4 years ago

Is that page accepting PR's from external authors?

You can do so here: https://github.com/dart-lang/site-www

mnordine commented 4 years ago

For pattern matching, read this: https://github.com/dart-lang/language/issues/546