Remove duplicated options from radio #230
Conversation
widget/radio_test.go
Outdated
| func TestRadio_DuplicatedOptions(t *testing.T) { | ||
| radio := NewRadio([]string{"Hi", "Hi", "Hi", "Another", "Another"}, nil) | ||
|
|
||
| Refresh(radio) |
There was a problem hiding this comment.
I don't think we should rely on this refresh call - there has been no specific change to a field so the developer would not know it is required. Perhaps the deduplication needs to be called from the constructor function as well?
|
|
||
| assert.Equal(t, 2, len(radio.Options)) | ||
| assert.Equal(t, 2, len(Renderer(radio).(*radioRenderer).items)) | ||
| } |
There was a problem hiding this comment.
Can you also test what happens if Append is called()?
widget/radio.go
Outdated
| @@ -20,6 +20,20 @@ type radioRenderer struct { | |||
| radio *Radio | |||
| } | |||
|
|
|||
| func removeDuplicatedOptions(options []string) []string { | |||
There was a problem hiding this comment.
I am not sure about the naming here. This function takes in, and returns, a slice of strings - it does not really know what "options" are...
Perhaps it should be a function on the radio type? (and then it would not need the input/output variables)?
Of perhaps it should have a name that represents it's generic nature such as "removeDuplicateStrings".
This function is currently available to the whole widget package - this may be a good thing as perhaps CheckBox should have the same filter?
I will leave the solution up to you - or chat on Slack :)
widget/radio.go
Outdated
| found := make(map[string]bool) | ||
|
|
||
| for _, option := range options { | ||
| if _, check := found[option]; !check { |
There was a problem hiding this comment.
This is more idiomatically if _, ok := found[option]; !ok { (unless "check" has some domain specific meaning)
Also suggesting the function name func dedupe([]string) []string
Girbons
force-pushed
the
radio-options-same-text
branch
from
5e99b34 to
9f31463
Compare
it should fix #213