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.

Sign up for GitHub

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

fix: valueOr and withValue utilities #1079

Merged
merged 3 commits into from
Apr 4, 2024
Merged

fix: valueOr and withValue utilities #1079

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0005d6
fix utilities
lchenut Apr 3, 2024
a40ba1a
improve tests
lchenut Apr 4, 2024
5081127
replace `check false` by `fail()` and simplify one test
lchenut Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions libp2p/utility.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,24 +112,27 @@ template withValue*[T](self: Opt[T] | Option[T], value, body: untyped): untyped
let value {.inject.} = temp.get()
body

macro withValue*[T](self: Opt[T] | Option[T], value, body, body2: untyped): untyped =
let elseBody = body2[0]
macro withValue*[T](self: Opt[T] | Option[T], value, body, elseStmt: untyped): untyped =
let elseBody = elseStmt[0]
quote do:
if `self`.isSome:
let `value` {.inject.} = `self`.get()
let temp = (`self`)
if temp.isSome:
let `value` {.inject.} = temp.get()
`body`
else:
`elseBody`

template valueOr*[T](self: Option[T], body: untyped): untyped =
if self.isSome:
self.get()
let temp = (self)
if temp.isSome:
temp.get()
else:
body

template toOpt*[T, E](self: Result[T, E]): Opt[T] =
if self.isOk:
let temp = (self)
if temp.isOk:
when T is void: Result[void, void].ok()
else: Opt.some(self.unsafeGet())
else: Opt.some(temp.unsafeGet())
else:
Opt.none(type(T))
86 changes: 86 additions & 0 deletions tests/testutility.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# This file may not be copied, modified, or distributed except according to
# those terms.

import options
import ./helpers
import ../libp2p/utility

Expand Down Expand Up @@ -71,3 +72,88 @@ suite "Utility":
test "unsuccessful safeConvert from uint to int":
check not (compiles do:
result: uint = safeConvert[int, uint](11.uint))

suite "withValue and valueOr templates":
type
TestObj = ref object
x: int

proc objIncAndOpt(self: TestObj): Opt[TestObj] =
self.x.inc()
return Opt.some(self)

proc objIncAndOption(self: TestObj): Option[TestObj] =
self.x.inc()
return some(self)

test "withValue calls right branch when Opt/Option is none":
var counter = 0
# check Opt/Option withValue with else
Opt.none(TestObj).withValue(v):
check false
lchenut marked this conversation as resolved.
Show resolved Hide resolved
else:
counter.inc()
Copy link
Contributor

Choose a reason for hiding this comment

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

we can probably extract it to a proc inside the test and avoid the repetition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean something like that?

proc testProc[T](val: Opt[T] | Option[T], counter: var int) =
  val.withValue(v):
    fail()
  else:
    counter.inc()
testProc(Opt.none(TestObj), counter)
testProc(none(TestObj), counter)

If there were more than two occurrences of this test I would do it, but I don't think it really brings something in this specific case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the benefit is avoiding code duplication, which I believe is never desirable.

none(TestObj).withValue(v):
check false
else:
counter.inc()
check counter == 2

# check Opt/Option withValue without else
Opt.none(TestObj).withValue(v):
check false
none(TestObj).withValue(v):
check false

test "withValue calls right branch when Opt/Option is some":
var obj = TestObj(x: 0)
lchenut marked this conversation as resolved.
Show resolved Hide resolved
# check Opt/Option withValue with else
Opt.some(obj).withValue(v):
v.x.inc()
else:
check false
some(obj).withValue(v):
v.x.inc()
else:
check false

# check Opt/Option withValue without else
Opt.some(obj).withValue(v):
v.x.inc()
some(obj).withValue(v):
v.x.inc()
check obj.x == 4

test "withValue calls right branch when Opt/Option is some with proc call":
var obj = TestObj(x: 0)
# check Opt/Option withValue with else
objIncAndOpt(obj).withValue(v):
v.x.inc()
else:
check false
objIncAndOption(obj).withValue(v):
v.x.inc()
else:
check false

# check Opt/Option withValue without else
objIncAndOpt(obj).withValue(v):
v.x.inc()
objIncAndOption(obj).withValue(v):
v.x.inc()

check obj.x == 8

test "valueOr calls with and without proc call":
var obj = none(TestObj).valueOr:
TestObj(x: 0)
check obj.x == 0
obj = some(TestObj(x: 2)).valueOr:
check false
return
check obj.x == 2

obj = objIncAndOpt(obj).valueOr:
check false
return
check obj.x == 3
Loading