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

feat: add filter_map() to @immut/list #1154

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

rami3l
Copy link
Contributor

@rami3l rami3l commented Oct 22, 2024

Mirrors OCaml's List.filter_map.

Implementation

  • immut/list/list.mbt: Added the filter_map function, which maps over the list and keeps all values for which the mapped result is Some(value).
  • immut/list/list.mbti: Declared the filter_map function in the list type implementation.

Testing

  • immut/list/list_test.mbt: Added test cases for the filter_map function to validate its behavior with both non-empty and empty lists.

@coveralls
Copy link
Collaborator

coveralls commented Oct 22, 2024

Pull Request Test Coverage Report for Build 3676

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 82.922%

Totals Coverage Status
Change from base Build 3675: 0.02%
Covered Lines: 4268
Relevant Lines: 5147

💛 - Coveralls

@rami3l
Copy link
Contributor Author

rami3l commented Oct 22, 2024

The CI failure is irrelevant to this change.

Copy link

peter-jerry-ye-code-review bot commented Oct 22, 2024

Based on the provided git diff output and the context of the MoonBit language, here are three potential issues or suggestions I've identified:

  1. Typo in Documentation Comment:

    • In the filter_map function documentation, there is a typo in the example output comment. The correct output should be of([4, 6, 3]) as indicated in the test case, but the comment says output: of([4, 6, 3]). Although this is not a functional error, it's important for documentation accuracy.
  2. Potential Redundancy in filter_map Implementation:

    • The filter_map function uses a loop with continue statements to build the result list. However, the use of acc.add(v) within the loop might be redundant since acc is being passed to the next iteration. This could potentially be simplified for better readability and efficiency.
  3. Inconsistent Use of @list in Test Case:

    • In the test case for filter_map, the list rs is explicitly typed as @list.T[Int] and initialized as @list.Nil. This is inconsistent with the way ls is defined without the @list prefix in its methods. While this doesn't affect the functionality, it introduces inconsistency in the code style within the test case.

These observations focus on improving code clarity, documentation accuracy, and consistency, which are crucial for maintainability and readability of the codebase.

@bobzhang
Copy link
Contributor

lgtm, can you run moon fmt to pass the ci

@bobzhang bobzhang merged commit 8064733 into moonbitlang:main Oct 23, 2024
13 checks passed
@rami3l rami3l deleted the feat/list-filter-map branch October 23, 2024 01:32
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.

3 participants