add between/3

[guregu]

This adds between/3 as discussed in #163.

Thanks for the advice about control flow! I tried refactoring it.
Let me know if I should make any changes and I will be happy to.

BTW, should I add this to the default predicates too?

[ichiban]

AWESOME!! There might be a weird behaviour on the edge case of low = high = math.MaxInt64 so could you check it, please?

Other than that, this is a superb contribution. Thanks!

[ichiban]

This is a minor point but it'd be better to flow to k(env).

The code reads well if the successful flow of control runs down the page, eliminating error cases as they arise.
https://go.dev/doc/effective_go#if

  *
  |
  +--> Bool(false)
  |
  +--> Error(err)
  |
  v
k(env)

[guregu]

Good point! I tweaked it a bit. Should we go further and put k(env) at the very end? I'm OK with either way.

[ichiban]

When low = high = math.MaxInt64, low+1 overflows and becomes a negative integer.

This could be solved by checking if low < high:

if low == high { // Since we've already checked low <= high.
	return Bool(false)
}
return Between(low+1, upper, value, k, env)

Or, for a better performance:

ks := make([]func(*Env) *Promise, 0, 2)
ks = append(ks, func(context.Context) *Promise {
	return Unify(value, low, k, env)
})
if low < high {
	ks = append(ks, func(context.Context) *Promise {
		return Between(low+1, upper, value, k, env)
	})
}
return Delay(ks...)

[guregu]

Thanks! I changed it to the better performance suggestion.
Before it would overflow but get caught by

if low > high {
	return Bool(false)
}

but now it should completely eliminate that dead end 👍

[ichiban]

This should be definitely in the default predicates! You can register it anywhere in New() (totally unordered).

prolog/interpreter.go

Line 23 in 66a3f9f

func New(in io.Reader, out io.Writer) *Interpreter {

[codecov-commenter]

Codecov Report

Merging #179 (be75a64) into main (66a3f9f) will decrease coverage by 0.02%.
The diff coverage is 91.17%.

@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
- Coverage   95.95%   95.92%   -0.03%     
==========================================
  Files          20       20              
  Lines        6083     6117      +34     
==========================================
+ Hits         5837     5868      +31     
- Misses        205      208       +3     
  Partials       41       41              

Impacted Files	Coverage Δ
engine/builtin.go	93.57% <91.17%> (-0.04%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66a3f9f...be75a64. Read the comment docs.

[guregu]

This should be definitely in the default predicates! You can register it anywhere in New() (totally unordered).

prolog/interpreter.go

Line 23 in 66a3f9f

func New(in io.Reader, out io.Writer) *Interpreter {

Awesome! I added it below Compare in builtin.go and interpreter.go because it seems kind of similar to Compare 🙂

[guregu]

There might be a weird behaviour on the edge case of low = high = math.MaxInt64 so could you check it, please?

Added a couple extra tests using MaxInt64 👍
Thanks for the review! I committed a few changes based on your comments.
Let me know if I missed something.

[guregu]

I added a few more tests. This should bring coverage up to 100% I think.