Skip to content

Commit 08cde4d

Browse files
apelisseJennifer Buckley
authored and
Jennifer Buckley
committed
Implement Compare methods
The current Less implementation has an exponential complexity. Replacing Less with a Compare method avoids that problem.
1 parent fa5e197 commit 08cde4d

File tree

2 files changed

+100
-50
lines changed

2 files changed

+100
-50
lines changed

value/less_test.go

+13
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,19 @@ func TestValueLess(t *testing.T) {
310310
if tt.b.Less(tt.b) {
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

+87-50
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
@@ -206,27 +243,28 @@ func (l *List) Equals(rhs *List) bool {
206243

207244
// Less compares two lists lexically.
208245
func (l *List) Less(rhs *List) bool {
246+
return l.Compare(rhs) == -1
247+
}
248+
249+
// Compare compares two lists lexically. The result will be 0 if l==rhs, -1
250+
// if l < rhs, and +1 if l > rhs.
251+
func (l *List) Compare(rhs *List) int {
209252
i := 0
210253
for {
211254
if i >= len(l.Items) && i >= len(rhs.Items) {
212255
// Lists are the same length and all items are equal.
213-
return false
256+
return 0
214257
}
215258
if i >= len(l.Items) {
216259
// LHS is shorter.
217-
return true
260+
return -1
218261
}
219262
if i >= len(rhs.Items) {
220263
// RHS is shorter.
221-
return false
264+
return 1
222265
}
223-
if l.Items[i].Less(rhs.Items[i]) {
224-
// LHS is less; return
225-
return true
226-
}
227-
if rhs.Items[i].Less(l.Items[i]) {
228-
// RHS is less; return
229-
return false
266+
if c := l.Items[i].Compare(rhs.Items[i]); c != 0 {
267+
return c
230268
}
231269
// The items are equal; continue.
232270
i++
@@ -280,6 +318,11 @@ func (m *Map) Equals(rhs *Map) bool {
280318

281319
// Less compares two maps lexically.
282320
func (m *Map) Less(rhs *Map) bool {
321+
return m.Compare(rhs) == -1
322+
}
323+
324+
// Compare compares two maps lexically.
325+
func (m *Map) Compare(rhs *Map) int {
283326
var noAllocL, noAllocR [2]int
284327
var morder, rorder []int
285328

@@ -325,28 +368,22 @@ func (m *Map) Less(rhs *Map) bool {
325368
for {
326369
if i >= len(morder) && i >= len(rorder) {
327370
// Maps are the same length and all items are equal.
328-
return false
371+
return 0
329372
}
330373
if i >= len(morder) {
331374
// LHS is shorter.
332-
return true
375+
return -1
333376
}
334377
if i >= len(rorder) {
335378
// RHS is shorter.
336-
return false
379+
return 1
337380
}
338381
fa, fb := &m.Items[morder[i]], &rhs.Items[rorder[i]]
339-
if fa.Name != fb.Name {
340-
// the map having the field name that sorts lexically less is "less"
341-
return fa.Name < fb.Name
382+
if c := strings.Compare(fa.Name, fb.Name); c != 0 {
383+
return c
342384
}
343-
if fa.Value.Less(fb.Value) {
344-
// LHS is less; return
345-
return true
346-
}
347-
if fb.Value.Less(fa.Value) {
348-
// RHS is less; return
349-
return false
385+
if c := fa.Value.Compare(fb.Value); c != 0 {
386+
return c
350387
}
351388
// The items are equal; continue.
352389
i++

0 commit comments

Comments
 (0)