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

Slice: Add GroupBy func #10

Merged
merged 1 commit into from Jan 2, 2022
Merged

Slice: Add GroupBy func #10

merged 1 commit into from Jan 2, 2022

Conversation

donutloop
Copy link
Collaborator

@donutloop donutloop commented Jan 2, 2022

Group by: split slice into two groups, applies on each slice element a
predicate func to categorize this element.

Changes

  • Add groub by func
  • Add test case for this func

@donutloop
Copy link
Collaborator Author

@duke-git please review

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2022

Codecov Report

Merging #10 (935c4f6) into main (94b1a1d) will increase coverage by 0.08%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   78.12%   78.21%   +0.08%     
==========================================
  Files          22       22              
  Lines        1472     1487      +15     
==========================================
+ Hits         1150     1163      +13     
- Misses        215      216       +1     
- Partials      107      108       +1     
Impacted Files Coverage Δ
slice/slice.go 79.30% <86.66%> (+0.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94b1a1d...935c4f6. Read the comment docs.

@donutloop donutloop force-pushed the groupBy branch 3 times, most recently from 1379c85 to 67cfa47 Compare January 2, 2022 11:37
Group by: split slice into two groups, applies on each slice element a
predicate func to categorize this element.

Changes

* Add groub by func
* Add test case for this func
@duke-git
Copy link
Owner

duke-git commented Jan 2, 2022

@duke-git please review

@donutloop ,thanks, GroupBy func is helpful.

@duke-git duke-git merged commit b4a49fc into duke-git:main Jan 2, 2022
@donutloop
Copy link
Collaborator Author

@duke-git Almost all functions that currently use reflection to achieve reusability can be migrated to generics (a feature of 1.18). Can we open a branch v2 and start to migrate from reflection to generics soon?

Ref: https://go.dev/blog/go1.18beta1

@duke-git
Copy link
Owner

duke-git commented Jan 6, 2022

@duke-git Almost all functions that currently use reflection to achieve reusability can be migrated to generics (a feature of 1.18). Can we open a branch v2 and start to migrate from reflection to generics soon?

Ref: https://go.dev/blog/go1.18beta1

@donutloop. Thanks, good suggestion. It's necessary to replace reflection into generic. I do have the plan. The question is that generic support in Go 1.18 Beta which is a preview release, not production release. From my point of view, it's unstable for now. will it change or has minor adjustments in future version go?My suggestions are as follows: please review.

  1. Can we migrate from reflection to generics when go has a production release for generics? (maybe couple months later, not sure)
  2. After we migrate generics, we still need to support interface (reflection) for users who use go version under v1.18.
    (v1.x.x for interface reflection, v2.x.x for generics)

@donutloop
Copy link
Collaborator Author

donutloop commented Jan 6, 2022

@duke-git

1.) I don't expect any major design changes because a beta API should already be in a frozen state. My recommendation, we can create a branch develop V2, and then start to develop the generic form of all affected functions.
2.) It definitely makes sense to not immediately drop the support of the reflection versions.

Let me know what you think

@duke-git
Copy link
Owner

duke-git commented Jan 7, 2022

migrated

@duke-git

1.) I don't expect any major design changes because a beta API should already be in a frozen state. My recommendation, we can create a branch develop V2, and then start to develop the generic form of all affected functions. 2.) It definitely makes sense to not immediately drop the support of the reflection versions.

Let me know what you think

@donutloop. Agree. I spent a couple of hours understand the go generics and try to rewrite some funcs (slice/slice.go, Map, Filter, ForEach). I find the code is concise and elegant after use generics, although generics syntax is a bit messy。I have checkout branch v2, we can migrate to generics in slice/slice.go step by step. How about it?

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.

None yet

3 participants