Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A case for a better data type or better maps #1659

Closed
cedvdb opened this issue May 31, 2021 · 20 comments
Closed

A case for a better data type or better maps #1659

cedvdb opened this issue May 31, 2021 · 20 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@cedvdb
Copy link

cedvdb commented May 31, 2021

Core idea

The idea is to have a new data type that is better suited for working with a type of data that has String keys and dynamic value than the current class or map mechanism. On top of that new data types, utilities are furnished to make them malleable.

A placeholder "data" is used for this data type:

data Product {
  String id;
  String name;
  bool favorite;
}

// utilities
Product a = { id: '1', name: 'test', favorite: true };
Partial<Product> b = { name: 'test' };
Nullable<Product> c = { id: '2', name: null, favorite: false };

// members access
print(a.id); // "1"
print(a.keys()); // ['id', 'name', 'favorite'];

// serialization
print(jsonEncode(a)); // {"id": "1", "name": "test", "favorite": true }

Motivations

  1. Type safety, I'm arguing here that Dart forces unsafe practices.
  2. Partial representation of the data
  3. Serialization
  4. Meta programing won't help

Classes data structure in Dart are often not suited for front-end development.

Most commonly, any application has to deal with this type of data:

  • Data that comes from a server, for most people it's always String keys with dynamic values (json, xml, etc).
  • Data has to be represented in various subsets. An User might have a set of 20 properties but in some part of an application we want only a subset of those properties. For example a Profile form value can't be represented by a Profile class, because it doesn't have the id property. An update on the Profile cannot be represented by the Profile class because it updates only a subset of the Profile properties.

Because of those drawbacks, it is sometimes more practical, to use Maps which is not type safe. The alternative, of using classes, is in some cases impractical to impossible, I will furnish clear examples after.

1. To be type safe

To show that the current way pushes you to write unsafe code, consider you are working on a collaborative application where you have a product details page. On that page some properties of the product can be updated by different users at the same time. You also took the choice, by design to never update your product in dart but rather just reflect the update that happen on the server.

Here is a model of the relevant code for the argument:

data Product {
  final String name;
  final bool favorite;
  // + 100 more properties
}

class ProductsAPI {
   // stream of products, changes on the backend are reflected here
   final products$ = firestore.collection('products').snapshots();
   
   update(Map<String, dynamic> updates, String id) {
     // updates the product on the backend
     return firestore.collection('products').doc(id).update(jsonEncode(updates));
   }

}

An update to the product could be any combination of fields of the product and you don't want users to override each others changes. Therefor the only sensible solution is to use a Map. This is not type safe. If you change a property name on your product class, without renaming the map key you now have a bug and a pretty substantial one as it could easily slips by testing and compilation will fail to spot it.

You could mitigate that by using an enum for the keys, and with the way enums work in dart you'll also have to map the enum values to the string names, which brings you to square one with the same vulnerabilities and a ton of boiler plate.

You could also mitigate this issue by building a diff system, where you'd copy the original product with a new property but then you need a diff mechanism to only send what you are updating to not override other people changes.

This is for me the biggest issue. Dart tries its hardest to spot mistakes on the compilation phase then drops the ball somewhere it really makes a difference.

With the proposal the code becomes:

class Product {
  final String name;
  final bool favorite;
  // + 100 more properties
}

class ProductsAPI {
   // stream of products, changes on the backend are reflected here
   final products$ = firestore.collection('products').snapshots();
   
   update(Product updates, String id) {
     // updates the product on the backend
     return firestore.collection('products').doc(id).update(updates);
   }

}

2. Partial representation of the data

(This is closely related to #1)

Reactive_forms is one of the most used package for forms in flutter. It currently has no way of representing it's data model in dart.

If you'd want a form to be typed one to one to a subset of your Product you'll end up with a Map<String, Object>.

  final formGroup = FormGroup({
    'firstName': FormControl<String>(),
    'lastName': FormControl<String>(),
  });
  final value = formGroup.value(); // Map<String, Object?>

The model is totally lost here. This type of scenario becomes hard to maintain once the application grows.

With a correct data type:

  final formGroup = FormGroup<Partial<Product>>({
    firstName: FormControl<String>(),
    lastName: FormControl<String>(),
  });
  final value = formGroup.value(); // Partial<Product>

3. Serialization

One common use case of such data structure is serialization. Having a data type where the keys are designed to be Strings would facilitate one of the biggest use of serializing to json

jsonEncode(product); // as you'd encode a Map<String, dynamic>

4. Meta programing won't help

I've come to the conclusion that meta programing is not a good solution for this because the underlying data structure simply does not exist.

argument 1:

Q: What will meta programing generate to solve the problem in #1 ?
A: I personally don't know, maybe one of the meta programming enthusiast here can answer. One thing I know though, is that if you generate a second class MutableProduct with every property being mutable or even nullable, you'll need a diff mechanism to prevent users from overriding each others changes.

argument 2:

While I'm not against meta programing, it hides the implementation from developers. Having such a generic and canonical feature as a core language feature is important for sharing the documentation and the same code. More so than third party libraries where every 5 devs use a different one.

@cedvdb cedvdb added the feature Proposed language feature that solves one or more problems label May 31, 2021
@lrhn
Copy link
Member

lrhn commented May 31, 2021

This reminds me very much about a database abstraction layer entity model from a prior job.

It was code-generated from a specification of the entity classes by their field names and types, and the entity objects could record, for each property, whether the property was set at all (if not, the object was partial, allowing you to request partial entries) and whether the property had been modified since "something" (usually since it was fetched from the server).
Only one class was needed per entity class, the flags were bit-fields (it was Java, so most likely a BitSet), with two bits per property, plus a strategy object per entity class with helper functions.

With that, you could do partial fetches, send only the modified properties back to the server, and merge changes from multiple operations (if you handle conflicts somehow).

All in all, I think it could do everything you describe here, with some code generation and two bits extra per field.

@gmpassos
Copy link

gmpassos commented Jun 1, 2021

Data classes also should have default values.

How about this?:

data Product {
  String id; // will be required
  String name = 'Unknown';
  bool favorite = false ;
  String? extra ; // nullable, optional, without a default value.
}

var a = Product{ id: '1', name: 'test', favorite: true };
var b = Product{ id: '2' }; // with the default values
var c = Product{ id: '3' , extra: 'foo' }; // defining the optional value.

@Levi-Lesches
Copy link

@gmpassos then the question becomes "how can you programmatically omit extra?" You can't just pass null to it, because it's nullable.

@cedvdb, is it fair to say that the difference between partial classes and classes with all-nullable fields is exactly that? The distinction between "undefined" and null? If so, there are many open issues trying to resolve the ambiguity, and I think once that's resolved we may not need partial classes anymore.

@gmpassos
Copy link

gmpassos commented Jun 1, 2021

I'm not thinking in partial data classes, or even if this is a needed feature. In my example a data class always have all fields. If you are not passing a field for a constructor it will have a default value, or null (if nullable), very similar to current named parameters usage when calling a method.

@cedvdb
Copy link
Author

cedvdb commented Jun 1, 2021

Levi yes and that distinction already kind of exist in maps, when it doesn't have a key. However there is no way to tell a map which key it should have or which key is optional (undefined) and so on.

So for the extra question, in this case, it wouldn't have the key when unspecified (like a map or like undefined). If it had a default value of null it would be null.

@Levi-Lesches
Copy link

Right, this is a big issue in Dart and I believe when (if?) it's fixed, this case should be a lot easier to deal with. There is some discussion in #877 for what it's worth

@cedvdb cedvdb changed the title A case for a better data type A case for a better data type or better maps Jun 1, 2021
@cedvdb
Copy link
Author

cedvdb commented Jun 3, 2021

To be honest I'm not sure about an undefined type.

Couldn't we have typed maps, were the potential keys are known by the compiler without resorting to an undefined value ? After all it already exist when the map does not have a specific key, it's just that the maps are not really typed and the keys don't figure in intellisens

@cedvdb
Copy link
Author

cedvdb commented Jun 3, 2021

@lrhn I've an hard time understanding your comment and its intent. Are you implying that this is doable today with some code generation and therefor not that useful ? (Not trying to put words in your mouth, the comment is just hard to decipher).

I'm sure there are ways to go around some of the issues I mentioned. The thing is that this kind of scenario is in most decent sized front end application. Having a core language that deals with this, cleanly and easily, is of the utmost necessity for a language that is primarily targeted at front-end (flutter). Also the solution seem unnecessary complex (I searched for BitSet and it is said it is a vector of bits) and doesn't handle serialization. For uses cases that are as canonical as the ones I mentioned, including serialization, it would not be sensible to have to maintain an additional layer. I'm sure people will just revert to using maps as they are doing currently and be subject to bugs from invalid String keys.

I could have totally misinterpreted your comment.

@Levi-Lesches
Copy link

To be honest I'm not sure about an undefined type.

Me neither. I don't like the idea of essentially adding another null, just wanted to bring up the issue that addresses the need clearly. But the underlying issue still presents itself with your typed maps. Consider the following:

void main() {
  // This TypedMap has the following types: 
  // - String name
  // - int price
  // - String? userReview
  final TypedMap full = {"name": "Toy", "price": 2, "userReview": null};
  final TypedMap partial = {"price": 3};  // change price to $3
  print(partial.userReview);  // what should be printed here?
}

In other words, if we try to ask "did the user just add a review?", what should the answer be? IMO, the only acceptable answer here would be to use .containsKey instead of checking for null. But then you're left with two problems:

  1. When checking the value of a nullable value, you'll get null if the key isn't in the map.
  2. How can you programmatically omit the value of a nullable field? Passing null won't help.

Adding typed maps won't solve this fundamental problem of the difference between a variable with no value (undefined) and a variable whose value is null.

@cedvdb
Copy link
Author

cedvdb commented Jun 3, 2021

I don't think I'm understanding your questions fully because to me it seems it would be the same as a regular map and you even mentioned containsKey yourself so I don't see the problem. Therefore it would just print null, as a regular map would do.

For 1. You can check containsKey to see if it was set to null or not set at all.
For 2. You just don't pass a value, it won't contain the key then and is therefore an analog to undefined.

I think I see where our view diverge now. In typescript (I don't know your background) they have interfaces that are applicable on json objects (although JavaScript has a Map type, I'll refer next to this json object as a map). The interface helps for the intellisens, to tell the développer what property can be set or accessed on the map (the json object).

However, and I think this is where our views differ, the interface never puts anything in the map, therefore a value is undefined until you put something in a specific key. Similarly in dart, my view is that a typed map would not put anything in the map from the get go and you'd be able to check if it containsKey or not.

If you have something that is marked as being a product it will need every property of the product map to be set. If something is marked as partial, well only some of the properties need to be set, both nullable properties and properties not set, when accessed will yield null. It already works like that with a regular map so I don't see that as an issue.

@Levi-Lesches
Copy link

Okay, so to clarify, accessing nullable values of a partially typed map can be a bad idea because you may get null. That may mean the value was either 1) not included or 2) set to null, so you must use .containsKey for checking and .remove for removing changes.

So how would this be resolved?

void main() {
  Map full = {"name": "Toy", "price": 2, "review": "SO COOL"};
  Map updates = {"price": 3, "review": null};  // $2 --> $3, and remove the user's review
  
  int newPrice = updates ["price"];  // 3
  String? newReview = updates ["review"];  // null. Ambiguous, but okay.
  String newName = updates ["name"];  // What is returned here?
}

This is where Dart and JS/TS diverge, since Dart has no concept of undefined. Instead, it returns null whenever a key doesn't exist in a Map. But if the map is typed, then it may be typed as non-nullable -- what should Dart do then?

@cedvdb
Copy link
Author

cedvdb commented Jun 4, 2021

Your example uses a Map it will return null, with a typed map it should not compile as is.

For my original example you give a partial typed map to the update function which encodes it to json. Since the properties are not even set the values won't be in the json and won't be transmitted over the network. Values that are set with a null values are transmitted.

@Levi-Lesches
Copy link

I think the disconnect here is the difference between literals and programmatically-constructed objects. It may be easy for Dart to inspect literals, but once an object is modified with logic, it's out of static analysis's reach. Consider the difference between these two:

void main() {
  const TypedMap updates = {"price": 3};
  /// If we really wanted to, Dart can tell at compile-time that this should be an error. 
  print(updates ["name"]);  
}
Future<void> main() async {
  final TypedMap updates = {"price": 3};
  if (await didNameChangeOnServer()) {  // unknown ahead of time
    updates ["name"] = "Game Console";
  }
  /// Dart can't tell if name has been added during compile-time
  /// If this is an error, it must be run-time, which is just as bad as not having type-safety at all.
  print(updates ["name"]); 
}

Phrased differently, Map is also typed to an extent -- it uses K and V. To get around this issue, Map.[] return V?. A TypedMap or any similar idea would have to do the same or risk throwing run-time errors.

@cedvdb
Copy link
Author

cedvdb commented Jun 5, 2021

Ahhh! That's where our misunderstanding was. My view was that if something is of a specific map type, then every property is assured to be set at initialization of said map. In your case it's after initialization. I believe it would make things easy but now I'm left wondering if that even makes sens. I guess pros and cons of both have to be weighted

@Levi-Lesches
Copy link

Levi-Lesches commented Jun 6, 2021

It's a tough issue, and the need for some distinction between null ("no value"/"N/A") and "not specified"/"undefined" comes up a lot (for example, in the context of a .copyWith method), and I really hope it's resolved nicely.

@cedvdb
Copy link
Author

cedvdb commented Jul 30, 2021

Thanks for the reply in the other discussion. Glad to hear meta programming can resolve this.

@Levi-Lesches
Copy link

Same here, and it was bothering me that it couldn't be done correctly. Glad to have that solved!

For archival purposes, here is the solution:

/// This is the original interface. We want to create a partial type of this class.
///
/// In this context, a partial type is one that remembers which of its properties are set. If a value has not
/// been set, then it won't include it in [json], and accessing it will throw a runtime error. See [PartialProduct] below.
@partial
class Product {
  final String name;
  final int price;
  final String? description;
  const Product(this.name, this.price, this.description);
  
  Map get json => {
    "name": name,
    "price": price,
    "description": description,
  };
}

/// This can be generated by metaprogramming.
///
/// Implements each field by maintaining a field `_hasProperty` for each property, and having the
/// properties themselves be `late` so they can keep their nullability. This means you can't specify 
/// values in the constructor, but rather must use setters for each field. This might even be preferable, 
/// since it allows you to change values later, at the small cost of using cascade syntax. 
///
/// However, this also means that final properties aren't really possible. The best you can do is use 
/// `late final` like I do here, which means that once the value is set, subsequent values are ignored.
///  But this approach won't generate compiler errors when you try to invoke the setter a second time. 
class PartialProduct implements Product {
  bool _hasName = false;
  late final String _name;
  void removeName() => _hasName = false;
  String get name => _name;
  set name(String value) {
    _hasName = true;
    _name = value;
  }

  bool _hasPrice = false;
  late final int _price;
  void removePrice() => _hasPrice = false;
  int get price => _price;
  set price(int value) {
    _hasPrice = true;
    _price = value;
  }

  bool _hasDescription = false;
  late final String? _description;
  void removeDescription() => _hasDescription = false;
  String? get description => _description;
  set description(String? value) {
    _hasDescription = true;
    _description = value;
  }
  
  Map get json => {
    if (_hasName)
      "name": name,
    if (_hasPrice)
      "price": price,
    if (_hasDescription)
      "description": description,
  };
}

void main() {
  Product product1 = PartialProduct()
    ..name = "toy";
  print(product1.json);  // {name: "toy"}
  
  Product product2 = PartialProduct()
    ..name = "toy"
    ..price = 3
    ..description = null;
  // Later, you can do: 
  product
    ..removePrice()  // remove variables without setting them to null
    ..product.name = "Fun Toy";  // silently ignore due to `late final`
  print(product2.json);  // {name: "toy", description: null}
}

@cedvdb
Copy link
Author

cedvdb commented Jan 14, 2022

I can't live without this anymore.

This is very very good that you managed to do this with the available tools but I now think something like this should be built-in the language because your version has the following disadvantage :

  • no constructor
  • no real final (as you mentioned), this is a deal breaker.

I hoped meta programing would solve this.. Maybe there is another feature that could do such a thing ?

It blows my mind that I'm alone asking for this, Flutter is a client sided language and CRUD operation is a thing in 99% of thoses.

@cedvdb cedvdb closed this as completed Jan 14, 2022
@cedvdb cedvdb reopened this Jan 14, 2022
@cedvdb
Copy link
Author

cedvdb commented Jan 15, 2022

I'm sure it will help for data classes. This issue however is more about partial classes than anything else. I failed to really pinpoint the issue when creating it so the tittle does not reflect the need.

Meta programing can also help for partial classes (as Levi posted a snippet that should work) even though it is not perfect, it can work.

One other area I heard people wanted to have meta programming was also flutter stateful widget. Maybe auto disposal of resources ?

@Levi-Lesches
Copy link

One other area I heard people wanted to have meta programming was also flutter stateful widget. Maybe auto disposal of resources ?

Yeah, @functionalWidget and @autoDispose are two widely-requested macros that are usually used as examples. I used both as examples in my static metaprogramming proposal, #1565. (Which is actually taking form a lot more than I thought it would, as the current implementation plan relies on generating "augmentations", analogous to the partial classes and part files system I described.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

4 participants