google / built_collection.dart

Immutable Dart collections via the builder pattern.
https://pub.dev/packages/built_collection
BSD 3-Clause "New" or "Revised" License
275 stars 52 forks source link

Rename Map.build() to Map.toBuiltMap() #256

Closed lukepighetti closed 2 years ago

lukepighetti commented 2 years ago

List extension is List.toBuiltList()

When trying to discover the Map equivalent, it's hard to find because it's Map.build().

This PR aliases Map.toBuiltMap() to Map.build() and deprecates the latter.

This will unify the naming convention with List.toBuiltList() and improve discoverability of the Map to BuiltMap converting extension

davidmorgan commented 2 years ago

The List extension is also called build; I think what you're referring to is that the Iterable extension is toBuiltList.

Map doesn't interact in the same way with Iterable, so there's no extension there for Map. We could add an extension on Iterable<MapEntry<K, V>> called toBuiltMap, I think that would be equivalent. But not sure how often people would find it. What do you think?

lukepighetti commented 2 years ago

OK, I understand better the situation now. Thank you.

I agree with your assessment given the current state of the codebase. And I agree that Iterable.toBuiltMap() is unlikely to be found or used often. I have changed this PR to match your suggestions, although the PR no longer addresses my core concerns, which are merely stylistic.

I believe that List.build() / Map.build() mentally conflict with BuiltList.build((b)=> ...) and BuiltMap.build((b)=> ...). I believe that this is confusing. I believe the API should only be:

Iterable.toBuiltList()
Iterable.toBuiltSet()
Map.toBuiltMap()

BuiltList.build((b)=>...)
BuiltSet.build((b)=>...)
BuiltMap.build((b)=>...)

And the following should be deprecated

List.build()
Set.build()
Map.build()
dave26199 commented 2 years ago

I understand your point of view-- but don't think it's worth changing at this point.

If you like you can add the renamed extension methods for use in your own code :)

Thanks.