Closed prgsmall closed 4 years ago
Will do. Thanks for the review.
On Mon, Jun 8, 2020 at 3:45 PM Elliot Chance notifications@github.com wrote:
@elliotchance requested changes on this pull request.
In go.mod https://github.com/elliotchance/orderedmap/pull/14#discussion_r436953306 :
@@ -1,4 +1,4 @@ -module github.com/elliotchance/orderedmap +module github.com/prgsmall/orderedmap
Revert this.
In orderedmap.go https://github.com/elliotchance/orderedmap/pull/14#discussion_r436956720 :
type OrderedMap struct {
- kv map[interface{}]*list.Element
- ll *list.List
- kv map[interface{}]*list.Element
- ll *list.List
- maxSize int
I don't want to pollute OrderedMap with features in this way. Instead you can create a new type that encapsulates the OrderedMap.
In orderedmap_test.go https://github.com/elliotchance/orderedmap/pull/14#discussion_r436958196 :
"github.com/stretchr/testify/assert"
)
-func TestNewOrderedMap(t *testing.T) {
- m := orderedmap.NewOrderedMap()
- assert.IsType(t, &orderedmap.OrderedMap{}, m) +func TestObjectCreation(t *testing.T) {
- t.Run("TestNewOrderedMap", func(t *testing.T) {
- m := orderedmap.NewOrderedMap()
- assert.IsType(t, &orderedmap.OrderedMap{}, m)
- assert.EqualValues(t, -1, m.Max())
- assert.EqualValues(t, false, m.IsFull())
nit: Use these assertions:
assert.Equal(t, -1, m.Max()) assert.False(t, m.IsFull())
In orderedmap.go https://github.com/elliotchance/orderedmap/pull/14#discussion_r436958379 :
@@ -60,6 +77,16 @@ func (m *OrderedMap) Len() int { return len(m.kv) }
+// Max returns the maximum size of the map +func (m *OrderedMap) Max() int {
Please use Capacity instead. It's more common with the terminology for go slices.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/elliotchance/orderedmap/pull/14#pullrequestreview-426559302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGU5B7G2IPDZ2AUSSGF3B3RVU5WNANCNFSM4NYQ3ZVQ .
-- Guerzenich-Small Consulting +1 505.216.6717
1929 Commerce St, Suite 7C Yorktown Heights, NY 10598
I'm going to create a new project which references your project and uses ordered map as the storage mechanism.
On the needs for a project that I am working on is to have a ring buffer map that would show a windows of parsed lines indexed by ids that are parsed from each line. I've modified ordered map to take an optional maxSize field, which can be set at creation time to ensure this behavior.
It add a new New... method and Max(), IsFull() methods, and modifies the Set to delete the Front() element if Max() has been met.
This change is![Reviewable](https://reviewable.io/review_button.svg)