Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Add Gt operator #103

Merged
merged 4 commits into from
Apr 3, 2019
Merged

Add Gt operator #103

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a185af4
Add Gt operator
Apr 2, 2019
35f7361
Rename to exprGT
Apr 2, 2019
8017a0d
Uppercase GT occurences
Apr 2, 2019
cbaafb7
Uppercase forgotten occurence of GT
Apr 2, 2019
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
79 changes: 79 additions & 0 deletions rule/expr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,46 @@ func (n *exprIn) Eval(params Params) (*Value, error) {
return BoolValue(false), nil
}

type exprGt struct {
yaziine marked this conversation as resolved.
Show resolved Hide resolved
operator
}

// Gt creates an expression that takes at least two operands and
// evaluates to true if each successive operand has a higher value than
// the next.
func Gt(v1, v2 Expr, vN ...Expr) Expr {
return &exprGt{
operator: operator{
kind: "gt",
operands: append([]Expr{v1, v2}, vN...),
},
}
}

func (n *exprGt) Eval(params Params) (*Value, error) {
if len(n.operands) < 2 {
return nil, errors.New("invalid number of operands in Gt func")
}

vA, err := n.operands[0].Eval(params)
if err != nil {
return nil, err
}

for i := 1; i < len(n.operands); i++ {
vB, err := n.operands[i].Eval(params)
if err != nil {
return nil, err
}

if !vA.Gt(vB) {
return BoolValue(false), nil
}
}

return BoolValue(true), nil
}

// Param is an expression used to select a parameter passed during evaluation and return its corresponding value.
type Param struct {
Kind string `json:"kind"`
Expand Down Expand Up @@ -382,6 +422,45 @@ func (v *Value) Equal(other *Value) bool {
return v.compare(token.EQL, other)
}

// Gt reports whether v is greater than other.
func (v *Value) Gt(other *Value) bool {
switch v.Type {
case "bool":
v1, _ := strconv.ParseBool(v.Data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why ignoring the error when you can return it?

Copy link
Author

Choose a reason for hiding this comment

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

Because the function won't fail, I am sure that I will parse the correct type.
It's impossible to have a value in v.Data that mismatches the type in v.Type.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if I create it by hand in the seeds scripts for example?

Copy link
Author

Choose a reason for hiding this comment

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

The only way to create Values is to use the rule.TValue functions (where T is a type) and those are strongly typed thus I don't see how we can have a value that differs from the type?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just being picky here, but Regula is a lib and you don't know how people might want to use it:

v := rule.Value{
  Type: "int64",
  Name: "foo",
  Data: fmt.Sprintf("%f", math.Pi / 2), // oups it's a float
}

Copy link
Author

Choose a reason for hiding this comment

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

From that perspective, you are right. But for this "intermediate" version I think that we can ship it like that and be more carefully in the next one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think intermediate versions allow us to be less careful about what we are producing. I agree that we should not make too much effort for nothing, but here we are talking about adding an if err != nil that doesn't cost much.

Copy link
Author

Choose a reason for hiding this comment

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

Regula is a lib and you don't know how people might want to use it

This argument is enough to add the check. From time to time I forgot that the users can do whatever they want with the exported type and my opinion is based on what they are doing and how they are using it at the moment.
Will add the check in #107.

if !v1 {
// If v1 is False then it's not greater than v2, and we can be done already.
return false
}

v2, _ := strconv.ParseBool(other.Data)
if v2 {
// If v2 is True then v1 can't be greater than it..
return false
}
return true
case "string":
if v.Data <= other.Data {
return false
}
return true
case "int64":
v1, _ := strconv.ParseInt(v.Data, 10, 64)
v2, _ := strconv.ParseInt(other.Data, 10, 64)
if v1 <= v2 {
return false
}
return true
case "float64":
v1, _ := strconv.ParseFloat(v.Data, 64)
v2, _ := strconv.ParseFloat(other.Data, 64)
if v1 <= v2 {
return false
}
return true
}
return false
}

type operander interface {
Operands() []Expr
}
Expand Down
73 changes: 73 additions & 0 deletions rule/expr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,79 @@ func TestIn(t *testing.T) {
})
}

func TestGt(t *testing.T) {
cases := []struct {
name string
m1 mockExpr
m2 mockExpr
expected *rule.Value
}{
{
name: "String: true",
m1: mockExpr{val: rule.StringValue("abd")},
m2: mockExpr{val: rule.StringValue("abc")},
expected: rule.BoolValue(true),
},
{
name: "String: false",
m1: mockExpr{val: rule.StringValue("abc")},
m2: mockExpr{val: rule.StringValue("abd")},
expected: rule.BoolValue(false),
},
{
name: "Bool: true",
m1: mockExpr{val: rule.BoolValue(true)},
m2: mockExpr{val: rule.BoolValue(false)},
expected: rule.BoolValue(true),
},
{
name: "Bool: false#1",
m1: mockExpr{val: rule.BoolValue(true)},
m2: mockExpr{val: rule.BoolValue(true)},
expected: rule.BoolValue(false),
},
{
name: "Bool: false#2",
m1: mockExpr{val: rule.BoolValue(false)},
m2: mockExpr{val: rule.BoolValue(true)},
expected: rule.BoolValue(false),
},
{
name: "Int64: true",
m1: mockExpr{val: rule.Int64Value(12)},
m2: mockExpr{val: rule.Int64Value(11)},
expected: rule.BoolValue(true),
},
{
name: "Int64: false",
m1: mockExpr{val: rule.Int64Value(12)},
m2: mockExpr{val: rule.Int64Value(12)},
expected: rule.BoolValue(false),
},
{
name: "Float64: true",
m1: mockExpr{val: rule.Float64Value(12.1)},
m2: mockExpr{val: rule.Float64Value(12.0)},
expected: rule.BoolValue(true),
},
{
name: "Float64: false",
m1: mockExpr{val: rule.Float64Value(12.0)},
m2: mockExpr{val: rule.Float64Value(12.1)},
expected: rule.BoolValue(false),
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
gt := rule.Gt(&tc.m1, &tc.m2)
val, err := gt.Eval(nil)
require.NoError(t, err)
require.Equal(t, tc.expected, val)
})
}
}

func TestParam(t *testing.T) {
t.Run("OK", func(t *testing.T) {
v := rule.StringParam("foo")
Expand Down
4 changes: 4 additions & 0 deletions rule/json.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ func unmarshalExpr(kind string, data []byte) (Expr, error) {
var or exprOr
e = &or
err = or.UnmarshalJSON(data)
case "gt":
var gt exprGt
e = &gt
err = gt.UnmarshalJSON(data)
default:
err = errors.New("unknown expression kind")
}
Expand Down