godux icon indicating copy to clipboard operation
godux copied to clipboard

Unsure about GetAllState

Open JackMordaunt opened this issue 8 years ago • 1 comments

Hi there, correct me if I'm wrong, but GetAllState wont actually 'copy' the state if it contains any reference types, thus you gain no immutability.

// GetAllState return a full copy of the current state.
func (s *Store) GetAllState() interface{} {
    ret := map[string]interface{}{}

    s.storeStateLock.RLock()
    for k, v := range s.storeState.state {
        ret[k] = v  // <- This will only copy the address of v if v is a map, slice, or interface.
    }
    s.storeStateLock.RUnlock()

    return ret  // <- If someone mutates this variable later on it can potentially mutate the original.
}

Perhaps use reflect.DeepCopy or similar for true immutability. Otherwise it would be good to document this potential side effect.

What are your thoughts?

JackMordaunt avatar Sep 16 '16 14:09 JackMordaunt

Good point! The concept about immutability is very important concerning Redux. In Golang you also need to account for race conditions.

gurre avatar Jun 08 '17 14:06 gurre