Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Introduce DefensiveMin and DefensiveMax #12550

Closed
ggwpez opened this issue Oct 24, 2022 · 2 comments · Fixed by #12554
Closed

Introduce DefensiveMin and DefensiveMax #12550

ggwpez opened this issue Oct 24, 2022 · 2 comments · Fixed by #12554
Labels
J0-enhancement An additional feature request. Z0-trivial Writing the issue is of the same difficulty as patching the code. Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.

Comments

@ggwpez
Copy link
Member

ggwpez commented Oct 24, 2022

Add traits to run min/max operations defensively. In some cases we use min/max but do not expect the else case to trigger. These traits would help with finding if these cases actually ever trigger.

Was discussing it with Ank4n: can you add a defensive_min() and defensive_max as well? I think there are a LOT of use cases where we do these operations defensively.

Originally posted by @kianenigma in #12515 (comment)

trait DefensiveMin:

  • fn defensive_min by using <=
  • fn defensive_strict_min by using <

trait DefensiveMax:

  • fn defensive_max by using >=
  • fn defensive_strict_max by using >
@ggwpez ggwpez added J0-enhancement An additional feature request. Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be. Z0-trivial Writing the issue is of the same difficulty as patching the code. labels Oct 24, 2022
@ggwpez ggwpez moved this to Backlog in Runtime / FRAME Oct 24, 2022
@dharjeezy
Copy link
Contributor

dharjeezy commented Oct 24, 2022

I want to pick this up @ggwpez
Can you give me an example of how this can be used like in this example [here] (https://github.com/paritytech/substrate/pull/12515/files#diff-a66e1f9bb70b6a5516c58534be76d45cf3339c5a56df4534ebfda632afb63bccR384) ?

@ggwpez
Copy link
Member Author

ggwpez commented Oct 25, 2022

Example being something like:

assert_eq!(10, 10_u32.defensive_min(11_u32)); // works
assert_eq!(10, 11_u32.defensive_min(10_u32)); // panics in tests

That is at-least how I imagine it. Should be possible in Rust hopefully. @dharjeezy

Repository owner moved this from Backlog to Done in Runtime / FRAME Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. Z0-trivial Writing the issue is of the same difficulty as patching the code. Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants