Skip to content

Conversation

bobzhang
Copy link
Contributor

No description provided.

Copy link

peter-jerry-ye-code-review bot commented May 29, 2025

eachi method signature should also be updated for error handling consistency

Category
Correctness
Code Snippet
fn[T] Array::eachi(Self[T], (Int, T) -> Unit) -> Unit
Recommendation
Update eachi signature to: fn[T] Array::eachi(Self[T], (Int, T) -> Unit?Error) -> Unit?Error
Reasoning
For consistency with the each method's error handling capability, eachi should also support error propagation in callbacks since they serve similar iteration purposes

Documentation for Array::each needs to be updated to reflect error handling

Category
Maintainability
Code Snippet
pub fn[T] Array::each(self : Array[T], f : (T) -> Unit?Error) -> Unit?Error
Recommendation
Add documentation explaining error propagation behavior and example with error handling
Reasoning
Current documentation doesn't mention the error handling capability of each, which is important for users to understand how errors are propagated

Test coverage could be improved for error scenarios

Category
Correctness
Code Snippet
test "each with error callback" { ... }
Recommendation
Add test cases for: 1) Multiple successful iterations 2) Error on last element 3) Empty array with error callback
Reasoning
Current tests only cover error on second element. More edge cases should be tested to ensure robust error handling

@bobzhang
Copy link
Contributor Author

cc @gmlewis now we have some support for error callbacks, feedback is welcome

@bobzhang bobzhang enabled auto-merge (rebase) May 30, 2025 03:53
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 6989

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.504%

Totals Coverage Status
Change from base Build 6988: 0.0%
Covered Lines: 8737
Relevant Lines: 9445

💛 - Coveralls

@bobzhang bobzhang merged commit 3d7040d into main May 30, 2025
12 checks passed
@bobzhang bobzhang deleted the hongbo/test_new branch May 30, 2025 03:59
@gmlewis
Copy link
Contributor

gmlewis commented May 30, 2025

cc @gmlewis now we have some support for error callbacks, feedback is welcome

Very nice, @bobzhang! Should the same feature be added to eachi?

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