[ 
https://issues.apache.org/jira/browse/THRIFT-3467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15228245#comment-15228245
 ] 

Tom Deering edited comment on THRIFT-3467 at 4/6/16 1:35 PM:
-------------------------------------------------------------

[~jensg], I understand [~creker]'s argument that if bool values are given, then 
one doesn't have to use the "comma, ok" idiom to check if an item is in the set 
(Go map). However, I'd give 3 counterarguments in reply:

1. When you read code where a map has non-empty values (eg of type bool), it is 
natural to assume that the values have meaning. But for a set encoding, the 
values have no meaning by definition, so this is misleading to a reader of your 
code.
2. The "comma, ok" idiom in Go is common and well-understood, and is the 
standard safe way to check if a map key is defined.
3. Storing unnecessary booleans as map values wastes a small (though not 
exorbitant) amount of memory.

In my opinion, bool map values for a set encoding are a misleading, unnecessary 
attempt to avoid using a standard Go idiom. One man's opinion.

FYI, in an initializer, one can use {} as shorthand for struct{}{}. That will 
help make your code snippet look less bad :)


was (Author: tdeering):
I understand [~creker]'s argument that if bool values are given, then one 
doesn't have to use the "comma, ok" idiom to check if an item is in the set (Go 
map). However, I'd give 3 counterarguments in reply:

1. When you read code where a map has non-empty values (eg of type bool), it is 
natural to assume that the values have meaning. But for a set encoding, the 
values have no meaning by definition, so this is misleading to a reader of your 
code.
2. The "comma, ok" idiom in Go is common and well-understood, and is the 
standard safe way to check if a map key is defined.
3. Storing unnecessary booleans as map values wastes a small (though not 
exorbitant) amount of memory.

In my opinion, bool map values for a set encoding are a misleading, unnecessary 
attempt to avoid using a standard Go idiom. One man's opinion.

FYI, in an initializer, one can use {} as shorthand for struct{}{}. That will 
help make your code snippet look less bad :)

> Go Maps for Thrift Sets Should Have Values of Type struct{} 
> ------------------------------------------------------------
>
>                 Key: THRIFT-3467
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3467
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Go - Compiler
>    Affects Versions: 0.9.3
>            Reporter: Tom Deering
>            Assignee: artem antonenko
>              Labels: golang
>
> Sets in Thrift are currently turned into maps with boolean values in Go. 
> Example:
> Thrift
> {code}
> namespace go bug
> service FooService{
>       void bar (1:set<string> foos)
> }
> {code}
> Go
> {code}
> func (p *FooServiceClient) Bar(foos map[string]bool) (err error) 
> {code}
> Boolean map values waste memory. Map values should be of the zero-byte 
> struct{} type.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to