Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why not use int, uint, float as return type of Int(), Uint(), Float() #85

Closed
fanybook opened this issue Aug 2, 2018 · 9 comments
Closed

Comments

@fanybook
Copy link

fanybook commented Aug 2, 2018

        Redis = redis.NewClient(&redis.Options{
            Addr:     Config.Get("redis.default.host").String(),
            Password: Config.Get("redis.default.password").String(),
            DB:       int(Config.Get("redis.default.database").Int()),
        })

when i use .Int() metod, also need to use int()

@tidwall
Copy link
Owner

tidwall commented Aug 2, 2018

It was a design decision that I made early on. I wanted to support 64-bit values but did not want the function names like .Float64() .Int64() .Uint64().

@fanybook
Copy link
Author

fanybook commented Aug 2, 2018

support 64-bit, should like int64(Config.Get("redis.default.database").Int())

hope use int, uint, float as return type

@tidwall
Copy link
Owner

tidwall commented Aug 2, 2018

int64 is what .Int() returns right now. You don't need to convert.

For example to convert to different types:

int       ->  int(Config.Get("redis.default.database").Int())
uint      ->  uint(Config.Get("redis.default.database").Uint())
int32     ->  int32(Config.Get("redis.default.database").Int())
uint32    ->  uint32(Config.Get("redis.default.database").Uint())
float32   ->  float32(Config.Get("redis.default.database").Float())

@fanybook
Copy link
Author

fanybook commented Aug 3, 2018

I know

My means is turning int as default
if i need int64, i can use int64

@tidwall
Copy link
Owner

tidwall commented Aug 3, 2018

I can’t change the return type of Int() at this point because doing so would cause a breaking change for current users of the library.

@fanybook
Copy link
Author

fanybook commented Aug 3, 2018

ok, i change them in.my local src

@tidwall
Copy link
Owner

tidwall commented Aug 6, 2018

sounds good. I'll close this issue if there isn't anything else to add. thanks.

@tidwall tidwall closed this as completed Aug 6, 2018
@Jake-Convictional
Copy link

Surprised by this design choice, I too find it unintuitive. Not a big deal though I suppose.

@tidwall
Copy link
Owner

tidwall commented Jun 27, 2022

It wasn't my finest hour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants