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

Add the reverseRangeFunc & rangeFunc methods on ArrayUtils contract to support decreasing ranges #17

Merged
merged 3 commits into from
May 18, 2023

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Apr 20, 2023

Examples:

var range = ArrayUtils.range(0, 10)
range == [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

range = ArrayUtils.reverseRange(10, 0)
range == [10, 9, 8, 7, 6, 5, 4, 3, 2, 1]

Update: Cadence tests for the ArrayUtils smart contract, were added:

flow test --cover test/*.cdc

Test results: "test/ArrayUtils_test.cdc"
- PASS: testRange
- PASS: testTransform
- PASS: testIterate
- PASS: testMap
- PASS: testMapStrings
- PASS: testReduce
Coverage: 100.0% of statements

@m-Peter
Copy link
Contributor Author

m-Peter commented Apr 26, 2023

cc @bjartek @bjartek @austinkline @turbolent
I have added the first unit test for ArrayUtils smart contract, using the Cadence Testing Framework 🙏
Let me know your thoughts!

@m-Peter m-Peter changed the title Update the rangeFunc method on ArrayUtils contract to support reversed range (declining) Update the rangeFunc method on ArrayUtils contract to support reversed range (decreasing) Apr 26, 2023
Copy link
Contributor

@austinkline austinkline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluesign thoughts on this? We might be better off defining a reverseRangeFunc so that users know this is an option. I would expect that end being less than start would panic but totally get the value of combining these.

@m-Peter m-Peter force-pushed the array-utils-reverse-range branch from ceb1c24 to 10a80d7 Compare April 27, 2023 08:30
@m-Peter m-Peter changed the title Update the rangeFunc method on ArrayUtils contract to support reversed range (decreasing) Add the reverseRangeFunc & rangeFunc methods on ArrayUtils contract to support decreasing ranges Apr 27, 2023
@m-Peter
Copy link
Contributor Author

m-Peter commented Apr 27, 2023

I have added separate methods for reverseRangeFunc and reverseRange, to be more explicit 🙏

@m-Peter
Copy link
Contributor Author

m-Peter commented May 5, 2023

@austinkline Kind reminder 🙏

@austinkline
Copy link
Contributor

@austinkline Kind reminder 🙏

Thank you!

@austinkline austinkline requested review from bjartek and austinkline May 5, 2023 15:17
The reverse function accepts an array as an argument, and returns
its reversed version.
Copy link
Contributor

@austinkline austinkline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but @bluesign @bjartek any other concerns or comments?

@bjartek
Copy link
Contributor

bjartek commented May 18, 2023

LGTM

Copy link
Contributor

@bluesign bluesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@turbolent
Copy link
Contributor

BTW, seeing the range helper functions in the array utils contract triggered this feature idea for the Cadence standard library: onflow/cadence#2482. Please chime in if you would like to see range functionality in Cadence itself!

@m-Peter
Copy link
Contributor Author

m-Peter commented May 18, 2023

That's nice! I'll add some feedback there too 🙏

@austinkline austinkline merged commit 2c8b180 into green-goo-dao:main May 18, 2023
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.

5 participants