Skip to content

Commit b45ef08

Browse files
committed
Implement Compare methods
The current Less implementation has an exponential complexity. Replacing Less with a Compare method avoids that problem.
1 parent 89db19a commit b45ef08

File tree

2 files changed

+113
-63
lines changed

2 files changed

+113
-63
lines changed

value/less_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,19 @@ func TestValueLess(t *testing.T) {
310310
if tt.b.Less(tt.a) {
311311
t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a)
312312
}
313+
314+
if tt.eq {
315+
if tt.a.Compare(tt.b) != 0 || tt.b.Compare(tt.b) != 0 {
316+
t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b)
317+
}
318+
} else {
319+
if !(tt.a.Compare(tt.b) < 0) {
320+
t.Errorf("oops, a is not less than b: %#v, %#v", tt.a, tt.b)
321+
}
322+
if !(tt.b.Compare(tt.a) > 0) {
323+
t.Errorf("oops, b is not more than a: %#v, %#v", tt.a, tt.b)
324+
}
325+
}
313326
})
314327
}
315328

value/value.go

Lines changed: 100 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -98,87 +98,124 @@ func (v Value) Equals(rhs Value) bool {
9898
// Less provides a total ordering for Value (so that they can be sorted, even
9999
// if they are of different types).
100100
func (v Value) Less(rhs Value) bool {
101+
return v.Compare(rhs) == -1
102+
}
103+
104+
// Compare provides a total ordering for Value (so that they can be
105+
// sorted, even if they are of different types). The result will be 0 if
106+
// v==rhs, -1 if v < rhs, and +1 if v > rhs.
107+
func (v Value) Compare(rhs Value) int {
101108
if v.FloatValue != nil {
102109
if rhs.FloatValue == nil {
103110
// Extra: compare floats and ints numerically.
104111
if rhs.IntValue != nil {
105-
return float64(*v.FloatValue) < float64(*rhs.IntValue)
112+
return v.FloatValue.Compare(Float(*rhs.IntValue))
106113
}
107-
return true
114+
return -1
108115
}
109-
return *v.FloatValue < *rhs.FloatValue
116+
return v.FloatValue.Compare(*rhs.FloatValue)
110117
} else if rhs.FloatValue != nil {
111118
// Extra: compare floats and ints numerically.
112119
if v.IntValue != nil {
113-
return float64(*v.IntValue) < float64(*rhs.FloatValue)
120+
return Float(*v.IntValue).Compare(*rhs.FloatValue)
114121
}
115-
return false
122+
return 1
116123
}
117124

118125
if v.IntValue != nil {
119126
if rhs.IntValue == nil {
120-
return true
127+
return -1
121128
}
122-
return *v.IntValue < *rhs.IntValue
129+
return v.IntValue.Compare(*rhs.IntValue)
123130
} else if rhs.IntValue != nil {
124-
return false
131+
return 1
125132
}
126133

127134
if v.StringValue != nil {
128135
if rhs.StringValue == nil {
129-
return true
136+
return -1
130137
}
131-
return *v.StringValue < *rhs.StringValue
138+
return strings.Compare(string(*v.StringValue), string(*rhs.StringValue))
132139
} else if rhs.StringValue != nil {
133-
return false
140+
return 1
134141
}
135142

136143
if v.BooleanValue != nil {
137144
if rhs.BooleanValue == nil {
138-
return true
139-
}
140-
if *v.BooleanValue == *rhs.BooleanValue {
141-
return false
145+
return -1
142146
}
143-
return *v.BooleanValue == false
147+
return v.BooleanValue.Compare(*rhs.BooleanValue)
144148
} else if rhs.BooleanValue != nil {
145-
return false
149+
return 1
146150
}
147151

148152
if v.ListValue != nil {
149153
if rhs.ListValue == nil {
150-
return true
154+
return -1
151155
}
152-
return v.ListValue.Less(rhs.ListValue)
156+
return v.ListValue.Compare(rhs.ListValue)
153157
} else if rhs.ListValue != nil {
154-
return false
158+
return 1
155159
}
156160
if v.MapValue != nil {
157161
if rhs.MapValue == nil {
158-
return true
162+
return -1
159163
}
160-
return v.MapValue.Less(rhs.MapValue)
164+
return v.MapValue.Compare(rhs.MapValue)
161165
} else if rhs.MapValue != nil {
162-
return false
166+
return 1
163167
}
164168
if v.Null {
165169
if !rhs.Null {
166-
return true
170+
return -1
167171
}
168-
return false
172+
return 0
169173
} else if rhs.Null {
170-
return false
174+
return 1
171175
}
172176

173177
// Invalid Value-- nothing is set.
174-
return false
178+
return 0
175179
}
176180

177181
type Int int64
178182
type Float float64
179183
type String string
180184
type Boolean bool
181185

186+
// Compare compares integers. The result will be 0 if i==rhs, -1 if i <
187+
// rhs, and +1 if i > rhs.
188+
func (i Int) Compare(rhs Int) int {
189+
if i > rhs {
190+
return 1
191+
} else if i < rhs {
192+
return -1
193+
}
194+
return 0
195+
}
196+
197+
// Compare compares floats. The result will be 0 if f==rhs, -1 if f <
198+
// rhs, and +1 if f > rhs.
199+
func (f Float) Compare(rhs Float) int {
200+
if f > rhs {
201+
return 1
202+
} else if f < rhs {
203+
return -1
204+
}
205+
return 0
206+
}
207+
208+
// Compare compares booleans. The result will be 0 if b==rhs, -1 if b <
209+
// rhs, and +1 if b > rhs.
210+
func (b Boolean) Compare(rhs Boolean) int {
211+
if b == rhs {
212+
return 0
213+
} else if b == false {
214+
return -1
215+
}
216+
return 1
217+
}
218+
182219
// Field is an individual key-value pair.
183220
type Field struct {
184221
Name string
@@ -207,31 +244,31 @@ func (f FieldList) Sort() {
207244

208245
// Less compares two lists lexically.
209246
func (f FieldList) Less(rhs FieldList) bool {
247+
return f.Compare(rhs) == -1
248+
}
249+
250+
// Less compares two lists lexically. The result will be 0 if f==rhs, -1
251+
// if f < rhs, and +1 if f > rhs.
252+
func (f FieldList) Compare(rhs FieldList) int {
210253
i := 0
211254
for {
212255
if i >= len(f) && i >= len(rhs) {
213256
// Maps are the same length and all items are equal.
214-
return false
257+
return 0
215258
}
216259
if i >= len(f) {
217260
// F is shorter.
218-
return true
261+
return -1
219262
}
220263
if i >= len(rhs) {
221264
// RHS is shorter.
222-
return false
265+
return 1
223266
}
224-
if f[i].Name != rhs[i].Name {
225-
// the map having the field name that sorts lexically less is "less"
226-
return f[i].Name < rhs[i].Name
267+
if c := strings.Compare(f[i].Name, rhs[i].Name); c != 0 {
268+
return c
227269
}
228-
if f[i].Value.Less(rhs[i].Value) {
229-
// F is less; return
230-
return true
231-
}
232-
if rhs[i].Value.Less(f[i].Value) {
233-
// RHS is less; return
234-
return false
270+
if c := f[i].Value.Compare(rhs[i].Value); c != 0 {
271+
return c
235272
}
236273
// The items are equal; continue.
237274
i++
@@ -259,27 +296,28 @@ func (l *List) Equals(rhs *List) bool {
259296

260297
// Less compares two lists lexically.
261298
func (l *List) Less(rhs *List) bool {
299+
return l.Compare(rhs) == -1
300+
}
301+
302+
// Compare compares two lists lexically. The result will be 0 if l==rhs, -1
303+
// if l < rhs, and +1 if l > rhs.
304+
func (l *List) Compare(rhs *List) int {
262305
i := 0
263306
for {
264307
if i >= len(l.Items) && i >= len(rhs.Items) {
265308
// Lists are the same length and all items are equal.
266-
return false
309+
return 0
267310
}
268311
if i >= len(l.Items) {
269312
// LHS is shorter.
270-
return true
313+
return -1
271314
}
272315
if i >= len(rhs.Items) {
273316
// RHS is shorter.
274-
return false
317+
return 1
275318
}
276-
if l.Items[i].Less(rhs.Items[i]) {
277-
// LHS is less; return
278-
return true
279-
}
280-
if rhs.Items[i].Less(l.Items[i]) {
281-
// RHS is less; return
282-
return false
319+
if c := l.Items[i].Compare(rhs.Items[i]); c != 0 {
320+
return c
283321
}
284322
// The items are equal; continue.
285323
i++
@@ -333,6 +371,11 @@ func (m *Map) Equals(rhs *Map) bool {
333371

334372
// Less compares two maps lexically.
335373
func (m *Map) Less(rhs *Map) bool {
374+
return m.Compare(rhs) == -1
375+
}
376+
377+
// Compare compares two maps lexically.
378+
func (m *Map) Compare(rhs *Map) int {
336379
var noAllocL, noAllocR [2]int
337380
var morder, rorder []int
338381

@@ -378,28 +421,22 @@ func (m *Map) Less(rhs *Map) bool {
378421
for {
379422
if i >= len(morder) && i >= len(rorder) {
380423
// Maps are the same length and all items are equal.
381-
return false
424+
return 0
382425
}
383426
if i >= len(morder) {
384427
// LHS is shorter.
385-
return true
428+
return -1
386429
}
387430
if i >= len(rorder) {
388431
// RHS is shorter.
389-
return false
432+
return 1
390433
}
391434
fa, fb := &m.Items[morder[i]], &rhs.Items[rorder[i]]
392-
if fa.Name != fb.Name {
393-
// the map having the field name that sorts lexically less is "less"
394-
return fa.Name < fb.Name
435+
if c := strings.Compare(fa.Name, fb.Name); c != 0 {
436+
return c
395437
}
396-
if fa.Value.Less(fb.Value) {
397-
// LHS is less; return
398-
return true
399-
}
400-
if fb.Value.Less(fa.Value) {
401-
// RHS is less; return
402-
return false
438+
if c := fa.Value.Compare(fb.Value); c != 0 {
439+
return c
403440
}
404441
// The items are equal; continue.
405442
i++

0 commit comments

Comments
 (0)