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

Accepting interface{} instead of []interface{} in BatchGetObjects #310

Open
sashayakovtseva opened this issue Jul 10, 2020 · 5 comments
Open

Comments

@sashayakovtseva
Copy link

sashayakovtseva commented Jul 10, 2020

Version: v3.0+

What we have now:

BatchGetObjects(policy *BatchPolicy, keys []*Key, objects []interface{}) (found []bool, err error)

So, to get objects into []myStruct one have to first get them into []interface{}, then convert each interface{} into myStruct.

I suggest:

BatchGetObjects(policy *BatchPolicy, keys []*Key, objects interface{}) (found []bool, err error)

This way one can pass []myStruct directly into BatchGetObjects. Inside BatchGetObjects reflect package can be used to check whether a slice type of appropriate length is passed.

As this is a breaking change, I suppose it may be implemented v4.0+.

@khaf
Copy link
Collaborator

khaf commented Jul 10, 2020

Makes sense. Will do.

@sashayakovtseva
Copy link
Author

sashayakovtseva commented Jul 10, 2020

Btw, I also think that accepting empty/nil slice and appending to it found object might be more convenient.

So that the final signature is the following:

BatchGetObjects(policy *BatchPolicy, keys []*Key, objects interface{}) error

That way package users won't have to iterate found []bool slice to find out what was actually fetched.
Not sure this is the expected API from aerospike client though, but definitely this is the common pattern I've seen a lot.

@khaf
Copy link
Collaborator

khaf commented Jul 10, 2020

That way you won't be able to reuse a pre-allocated slice.

@sashayakovtseva
Copy link
Author

you will be able, when truncating it to zero length. That way append won't allocate memory, but reuse underlying slice's capacity.

@khaf
Copy link
Collaborator

khaf commented Jul 10, 2020

ah, you literally meant append. That won't work due to race condition. We can only set at index without race, not append.

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

2 participants