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 extra tests / examples of lockable ranges #58

Merged
merged 4 commits into from
Jun 19, 2023
Merged

Conversation

maurolacy
Copy link
Collaborator

@maurolacy maurolacy commented Jun 17, 2023

Some more tests / examples of using our new Lockable primitive in range iterators.

Found useful writing these to reason about and learn how to use them.

  • Range iterator returning a collection.
  • Locked values re-mapping.
  • Locked values skipping.
  • Locked values reporting.

@maurolacy maurolacy requested a review from ethanfrey June 17, 2023 13:56
Add range iterator example
Add locked values re-mapping example
Add locked values skipping example
Copy link
Collaborator

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

LGTM

.unwrap_err();
assert_eq!(err, TestsError::Lock(LockError::WriteLocked));

// But we can re-map (perhaps not a good idea) the write-locked values
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to explain what is and is not possible here

@ethanfrey ethanfrey merged commit 7a35a54 into main Jun 19, 2023
2 checks passed
@ethanfrey ethanfrey deleted the extra-lockable-tests branch June 19, 2023 19:14
Copy link
Collaborator

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

One more idea after merging...

.unwrap();
assert_eq!(collaterals.len(), 3);

// Or we can skip (perhaps not a good idea either) the write-locked values
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reflecting more on this, I think the best would be to return:

enum MaybeAccount {
  Account(Account),
  Locked{ user: String },
}

Or something Similar. (Not sure how you use the keys).
I can make an example as follow up PR (unless this makes sense to you and you want to try)

Copy link
Collaborator Author

@maurolacy maurolacy Jun 20, 2023

Choose a reason for hiding this comment

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

👍🏼 Will implement this for reference / as an example.

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.

2 participants