Skip to content

Comments

Silexcorp dart3#10

Open
evant wants to merge 4 commits intomainfrom
silexcorp-dart3
Open

Silexcorp dart3#10
evant wants to merge 4 commits intomainfrom
silexcorp-dart3

Conversation

@evant
Copy link
Owner

@evant evant commented Aug 24, 2023

No description provided.

@evant evant mentioned this pull request Aug 24, 2023
name: streamqflite
description: reactive stream wrapper around sqflite inspired by sqlbrite
version: 1.0.0
version: 2.2.3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why 2.2.3? I guess to match sqflite? I don't think we need to actually track that closely with versions so 2.0.0 would be fine.

Comment on lines +10 to +11
Database? db;
StreamDatabase? streamDb;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can declare these as late to avoid the unwrapping latter https://dart.dev/null-safety/understanding-null-safety#late-variables

return _source.asyncMap((query) => query()).map((rows) {
var result = List<T>(rows.length);
//var result = List<T>(rows.length);
List<T> result = [];
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the admittedly minor optimization of sizing the list so it doesn't have to reallocate, just curious if there's a way to do that now, maybe by defining a capacity?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream<List<T>> mapToList<T>(T mapper(Map<String, dynamic> row)) {
return _source.asyncMap((query) => query()).map((rows) {
var result = List<T>(rows.length);
//var result = List<T>(rows.length);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to remove commented out code

@silexcorp
Copy link

Thank you :D I'll keep it in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants