-
Notifications
You must be signed in to change notification settings - Fork 7
Implemented functions for FlatList module to correspond to Array module
#10
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
base: develop
Are you sure you want to change the base?
Implemented functions for FlatList module to correspond to Array module
#10
Conversation
- conversions - plain distinct based on distinctBy - scans and folds - signatures simplifications - fix unzip3 - sorting/comparing
xperiandri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом хорошо.
Сравни как в модуле List там кажись все фкнции принимают строго list но не seq.
Соответственно тебе нужно будет добавить аннотации
xperiandri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну раз raiseOrReturn уже устанавливает такое ограничение, то супер
- remove excessive `get` - add initialization with value (and use in `create`/`replicate`) - use `Seq` in majority of functions - simplify `fill` - fix signatures
- change `countBy`, `indexed`, `iter2` to use `Seq` - fix distinctBy (remove excessive arg + use `raiseOrReturn`)
xperiandri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А так всё супер
FlatList module to correspond to Array module
| [<CompiledName("Build")>] | ||
| let inline build f = | ||
| let builder = builder() | ||
| f builder | ||
| moveFromBuilder builder | ||
|
|
||
| [<CompiledName("Update")>] | ||
| let inline update f list = | ||
| let builder = toBuilder list | ||
| f builder | ||
| moveFromBuilder builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А что эти функции делают?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Позволяют выполнить действие над билдером. Использований в модуле нет, думаю удалить их
|
Надо будет все вызовы переделать на вызовы отсюда У них это единственный класс, который имеет свой LINQ. |
|
Потому, что вместо проверки на null там проверяется, есть ли внутри ImmutableArray сам массив. |
|
|
||
| let inline internal checkNotDefault argName (list : FlatList<'T>) = | ||
| if list.IsDefault then invalidArg argName "Uninstantiated ImmutableArray/FlatList" | ||
| let inline internal check (list : FlatList<'T>) = checkNotDefault (nameof list) list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Получается самый быстрый вариант – это дёрнуть IsEmpty и выбросить
https://github.com/dotnet/runtime/blob/54c717a4ed822f46a23893479b8d4398596c041d/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Minimal.cs#L157
И получится то же, что они делают. Тогда не нужно использовать их LINQ обёртку
| if list.IsDefault then invalidArg argName "Uninstantiated ImmutableArray/FlatList" | ||
| let inline internal check (list : FlatList<'T>) = checkNotDefault (nameof list) list | ||
| let inline internal checkEmpty (list : FlatList<_>) = check list; if list.Length = 0 then invalidArg (nameof list) "Source is empty" else () | ||
| let inline internal raiseOrReturn list = check list; list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут наверное надо переименовать будет на checkAndReturn чтобы было в едином стиле
|
Посмотрел ещё раз https://github.com/dotnet/runtime/blob/main/src/libraries/System.Collections.Immutable/src/System/Linq/ImmutableArrayExtensions.cs |
|
При чём, наверное, именно для этой коллекции стоит отойти от правила возвращать такой же тип при преобразовании, а возвращать |
- rename checkAndReturn - use ImmutableArrayExtensions in some places - remove ofSeq calls
No description provided.