From e0d76485ec43034af0c355d6e0a974c8081acb39 Mon Sep 17 00:00:00 2001 From: wenchyzhu Date: Fri, 10 Jul 2026 15:51:18 +0800 Subject: [PATCH 1/2] feat(tableaupb): add Fraction.MatchAny/MatchAll and rename Cmp to Match Add two multi-comparator helpers on *Fraction: - MatchAny(cmps...): OR logic, returns false for an empty list (matches slices.ContainsFunc semantics). - MatchAll(cmps...): AND logic, returns true for an empty list (vacuous truth, the identity element for AND). Also rename Fraction.Cmp to Match for a consistent naming family (Match / MatchAny / MatchAll), and deprecate the package-level Compare as a thin wrapper over Match. Why Match instead of Cmp/Compare: - Go's Compare/Cmp idiom returns an int ordering (-1/0/+1), e.g. bytes.Compare, strings.Compare, bytes.(*Buffer).Cmp, cmp.Compare, and the slices.SortFunc comparator. A bool-returning function named Compare/Cmp misleads readers into expecting an int, and obscures why Cmp is even usable as a slices.ContainsFunc predicate. - The operation is a predicate test (does this value satisfy this constraint), not a peer-to-peer comparison: the operands are asymmetric (a Fraction value vs a Comparator constraint). - Match is an established Go bool-verb (regexp.MatchString) that reads as a test at both the method (f.Match(cmp)) and package-func levels. Co-Authored-By: Claude Opus 4.8 --- proto/tableaupb/wellknown_util.go | 25 ++++++- proto/tableaupb/wellknown_util_test.go | 91 +++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/proto/tableaupb/wellknown_util.go b/proto/tableaupb/wellknown_util.go index d9602f61..c0eab289 100644 --- a/proto/tableaupb/wellknown_util.go +++ b/proto/tableaupb/wellknown_util.go @@ -1,5 +1,7 @@ package tableaupb +import "slices" + // NewFraction creates a new fraction. func NewFraction(num, den int32) *Fraction { return &Fraction{Num: num, Den: den} @@ -29,11 +31,13 @@ func NewIntegerComparator(sign Comparator_Sign, num int32) *Comparator { } // Compare returns true if the given fraction matches the given comparator. +// +// Deprecated: Use [Fraction.Match] instead. func Compare(left *Fraction, cmp *Comparator) bool { - return left.Cmp(cmp) + return left.Match(cmp) } -func (f *Fraction) Cmp(cmp *Comparator) bool { +func (f *Fraction) Match(cmp *Comparator) bool { other := cmp.GetValue() // cross-multiply to compare lval := int64(f.GetNum()) * int64(other.GetDen()) @@ -55,3 +59,20 @@ func (f *Fraction) Cmp(cmp *Comparator) bool { panic("invalid compare operator") } } + +// MatchAny reports whether the fraction matches any of the given comparators (OR logic). +// It returns false if no comparators are provided. +func (f *Fraction) MatchAny(cmps ...*Comparator) bool { + return slices.ContainsFunc(cmps, f.Match) +} + +// MatchAll reports whether the fraction matches all of the given comparators (AND logic). +// It returns true if no comparators are provided (vacuous truth). +func (f *Fraction) MatchAll(cmps ...*Comparator) bool { + for _, cmp := range cmps { + if !f.Match(cmp) { + return false + } + } + return true +} diff --git a/proto/tableaupb/wellknown_util_test.go b/proto/tableaupb/wellknown_util_test.go index a253c59e..d314725e 100644 --- a/proto/tableaupb/wellknown_util_test.go +++ b/proto/tableaupb/wellknown_util_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -func TestCompare(t *testing.T) { +func TestMatch(t *testing.T) { type args struct { left *Fraction cmp *Comparator @@ -74,8 +74,93 @@ func TestCompare(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := Compare(tt.args.left, tt.args.cmp); got != tt.want { - t.Errorf("Compare() = %v, want %v", got, tt.want) + if got := tt.args.left.Match(tt.args.cmp); got != tt.want { + t.Errorf("Match() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchAny(t *testing.T) { + left := NewFraction(1, 2) + tests := []struct { + name string + cmps []*Comparator + want bool + }{ + { + name: "empty", + cmps: nil, + want: false, + }, + { + name: "any match (first)", + cmps: []*Comparator{ + NewComparator(Comparator_SIGN_EQUAL, 1, 2), // 1/2 == 1/2 → true + NewComparator(Comparator_SIGN_LESS, 1, 4), // 1/2 < 1/4 → false + }, + want: true, + }, + { + name: "any match (last)", + cmps: []*Comparator{ + NewComparator(Comparator_SIGN_LESS, 1, 4), // 1/2 < 1/4 → false + NewComparator(Comparator_SIGN_GREATER, 1, 4), // 1/2 > 1/4 → true + }, + want: true, + }, + { + name: "no match", + cmps: []*Comparator{ + NewComparator(Comparator_SIGN_LESS, 1, 4), // 1/2 < 1/4 → false + NewComparator(Comparator_SIGN_GREATER_OR_EQUAL, 3, 4), // 1/2 >= 3/4 → false + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := left.MatchAny(tt.cmps...); got != tt.want { + t.Errorf("MatchAny() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchAll(t *testing.T) { + left := NewFraction(1, 2) + tests := []struct { + name string + cmps []*Comparator + want bool + }{ + { + name: "empty (vacuous truth)", + cmps: nil, + want: true, + }, + { + name: "all match", + cmps: []*Comparator{ + NewComparator(Comparator_SIGN_EQUAL, 2, 4), // 1/2 == 2/4 → true + NewComparator(Comparator_SIGN_GREATER, 1, 4), // 1/2 > 1/4 → true + NewComparator(Comparator_SIGN_LESS_OR_EQUAL, 3, 4), // 1/2 <= 3/4 → true + }, + want: true, + }, + { + name: "one fails", + cmps: []*Comparator{ + NewComparator(Comparator_SIGN_EQUAL, 2, 4), // 1/2 == 2/4 → true + NewComparator(Comparator_SIGN_LESS, 1, 4), // 1/2 < 1/4 → false + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := left.MatchAll(tt.cmps...); got != tt.want { + t.Errorf("MatchAll() = %v, want %v", got, tt.want) } }) } From 15caf416860e02d2a43ee26cd8236eebeff14f85 Mon Sep 17 00:00:00 2001 From: wenchyzhu Date: Fri, 10 Jul 2026 20:28:00 +0800 Subject: [PATCH 2/2] refactor(tableaupb): document Fraction.Match and handle unknown sign MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a doc comment to Match describing the relational contract and why comparison uses cross-multiplication: exact integer arithmetic gives both performance (no division, no FP) and accuracy (no rounding on large int32 values that a/b-vs-c/d float comparison would risk). - Replace the default panic with returning false for an unknown Comparator_Sign. Proto enums are open-ended: a deserialized message can carry a sign a newer producer added. As a predicate feeding validation, Match degrades to "not satisfied" rather than crashing the load path — consistent with other enum-switch defaults in the repo (e.g. log/core/level.go) and with protovalidate's unknown-enum semantics. - Add an "unknown sign" test case pinning the new contract. Co-Authored-By: Claude Opus 4.8 --- proto/tableaupb/wellknown_util.go | 8 ++++++-- proto/tableaupb/wellknown_util_test.go | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/proto/tableaupb/wellknown_util.go b/proto/tableaupb/wellknown_util.go index c0eab289..b48df6ed 100644 --- a/proto/tableaupb/wellknown_util.go +++ b/proto/tableaupb/wellknown_util.go @@ -37,9 +37,12 @@ func Compare(left *Fraction, cmp *Comparator) bool { return left.Match(cmp) } +// Match reports whether the fraction satisfies the comparator's +// relational condition (==, !=, <, <=, >, >=). Comparison is done by +// cross-multiplication to stay in exact integer arithmetic for performance +// and accuracy. func (f *Fraction) Match(cmp *Comparator) bool { other := cmp.GetValue() - // cross-multiply to compare lval := int64(f.GetNum()) * int64(other.GetDen()) rval := int64(other.GetNum()) * int64(f.GetDen()) switch cmp.GetSign() { @@ -56,7 +59,8 @@ func (f *Fraction) Match(cmp *Comparator) bool { case Comparator_SIGN_GREATER_OR_EQUAL: return lval >= rval default: - panic("invalid compare operator") + // Unknown sign is treated as no-match; proto enums are open-ended (forward-compat), not a bug. + return false } } diff --git a/proto/tableaupb/wellknown_util_test.go b/proto/tableaupb/wellknown_util_test.go index d314725e..c07f6e53 100644 --- a/proto/tableaupb/wellknown_util_test.go +++ b/proto/tableaupb/wellknown_util_test.go @@ -71,6 +71,14 @@ func TestMatch(t *testing.T) { }, want: true, }, + { + name: "unknown sign", + args: args{ + left: NewFraction(1, 2), + cmp: NewComparator(Comparator_Sign(42), 1, 2), // unrecognized sign → no match + }, + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {