1
\$\begingroup\$

I made a struct that will loop through a list, and return the next element on calls to the Get() method. It also allows to remove elements from the list by calling the Evict() method, and is safe for concurrent use.

I'd appreciate any feedback: what's ok, what's not, what could be improved or changed.

package generators

import (
    "errors"
)

var (
    ErrEmpty    = errors.New("no elements were given")
    ErrConflict = errors.New("found the same element twice")
)

// InfiniteWithEvict will circle through a list of elements until cancelled,
// allowing to remove elements if needed. It is safe for concurrent use.
// It needs to be created with NewInfiniteWithEvict(),
// started with
// InfiniteWithEvict.Start()
// and cancelled by calling the function returned by InfiniteWithEvict.Start().
type InfiniteWithEvict struct {
    elems   []interface{}
    evictC  chan interface{}
    getC    chan interface{}
    indices map[interface{}]int
}

// NewInfiniteWithEvict returns an InfiniteWithEvict object.
// The contents of the slice elems will be modified by calls to Evict() method.
func NewInfiniteWithEvict(elems []interface{}) (*InfiniteWithEvict, error) {
    if len(elems) == 0 {
        return nil, ErrEmpty
    }
    m := make(map[interface{}]int)
    for idx, elem := range elems {
        if _, ok := m[elem]; ok {
            return nil, ErrConflict
        }
        m[elem] = idx
    }
    return &InfiniteWithEvict{
        elems:   elems,
        evictC:  make(chan interface{}),
        getC:    make(chan interface{}),
        indices: m,
    }, nil
}

// Start starts the generator.
// Before calling start, calls to Get() and Evict() will block.
// It returns a function that must be called when you are sure that
// no more calls to Get() or Evict() will be made.
func (p *InfiniteWithEvict) Start() func() {
    cancel := make(chan struct{})
    go p.loop(cancel)
    return func() {
        cancel <- struct{}{}
    }
}

// Get returns the next element of the list.
func (p *InfiniteWithEvict) Get() interface{} {
    return <-p.getC
}

// Evict removes an element from the list.
func (p *InfiniteWithEvict) Evict(v interface{}) {
    p.evictC <- v
}

func (p *InfiniteWithEvict) loop(cancel <-chan struct{}) {
    i := 0
    for {
        select {
        case <-cancel:
            close(p.getC)
            return
        case v := <-p.evictC:
            index, ok := p.indices[v]
            if !ok {
                continue
            }
            p.elems[index] = p.elems[len(p.elems)-1]
            p.elems = p.elems[:len(p.elems)-1]
            delete(p.indices, v)
            if len(p.elems) == 0 {
                continue
            }
            p.indices[p.elems[index]] = index
            if i == len(p.elems) {
                i = 0
            }
        case p.getC <- func() interface{} {
            if len(p.elems) == 0 {
                return nil
            }
            return p.elems[i]
        }():
            if i == len(p.elems)-1 {
                i = 0
            } else {
                i = i + 1
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ that case p.getC is painful reading. dont forget us when writing code, simpler is better. \$\endgroup\$ – mh-cbon Mar 27 at 6:30
  • \$\begingroup\$ the start function could be a private call from within the constructor call (who will not call start of this code when being used ?). On the other hand, a Done function gives end user the chance to kill it when needed. just use context.Context instead of non standard cancellation mechanism, IMHO. the p.getC case is racy. it consumes i and p.elems asynchronously. \$\endgroup\$ – mh-cbon Mar 27 at 6:36
  • \$\begingroup\$ write down some tests, with concurrent cases, run them with -race flags, see what happens. \$\endgroup\$ – mh-cbon Mar 27 at 6:37
  • \$\begingroup\$ I honestly struggle to see the benefit of your approach. Why not simply use a sync.Map, or a simple wrapper around a slice or map with a sync.RWMutex to ensure safe concurrent use? \$\endgroup\$ – Elias Van Ootegem Apr 2 at 15:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.