CODEWITHSUNDEEP
gochannel
unsafe_where_true
unsafe_where_true

Reputation: 6300

go: filter events in select and break

This is the code I have so far:

for {
    select {
    case event := <-events:
        if event.Msg != nil && len(activeFilters) > 0 {
            //filter for messages
            matches := false
            for k, v := range activeFilters {
                if k == event.Msg.Namepace {                        
                  for _, code := range v {
                    if code != event.Msg.Code {
                        matches = true    
                        break //BREAK1
                    }
                  }
                  if matches {
                    break //BREAK2
                  }
                }
            }
            if !matches {
              break //BREAK3
            }
        }
        if err := sendEvent(event); err != nil {
            sendErr(err)
            return
        }
    case <-closed:
        return
    }
}

First, I am a beginner in Go. The above code generally looks too complex for me, I guess it can be optimized.

What I want to do: when an event arrives, I want to check if a given Code is inside a list of filters. The filters have two "dimensions": a namespace and a list of codes. activeFilters is a map[string][]uint64. (Note: I am allowed to redesign activeFilters to some other structure).

So I just want to prevent the code to execute sendEvent if the filter doesn't match, while sendEvent should be executed if the protocol matches any key in activeFilters AND the message code is contained in the value array for that key.

So can the above code be simplified? Is the usage of break (BREAK1 and BREAK2) correct? BREAK2 seems to be needed for me because the inner for loop may already find a match, in which case there's no need to continue iterating on activeFilters.

And finally, am I correct in using BREAK3? My thinking there is that if there is no match, I do not want to execute sendEvent, so I want to jump out of the case. In my limited Go understanding, using return there would close the channel or something like that....

:blush

Upvotes: 0

Views: 545

Answers (1)

Adrian
Adrian

Reputation: 46442

If activeFilters is a map, you don't need to iterate it to find a key - that's the whole point of a map. You can just:

if v,ok := activeFilters[event.Msg.Namepace]; ok {
    // range over v as you are now
} else {
    // event.Msg.Namepace does not exist in the map
}

This eliminates one loop, and thus one break. The last break (BREAK 3) works but isn't particularly clear what it's doing; I'd make it a conditional to make it more readable:

if matches {
    if err := sendEvent(event); err != nil {
        sendErr(err)
        return
    }
}

return would not close the channel (assuming there's another reference to it somewhere else, which there would have to be for this to work at all), but it would exit your loop, so no further messages would be processed. I'd also double-check that the return in the error case is really what you want; typically in these situations you'd log the error but keep handling future messages that come over the channel.

Upvotes: 1

Related Questions