change the zadd method to accept Map<String, Double> #331

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@sing1ee
sing1ee commented Aug 14, 2012

I change the zadd method, and update the test.

@xetorthio
Owner

Can you please resend the pull request without all the formatting changes? It is very very hard to follow the changes, as you see like everything has changed.
If using eclipse, make sure you have set 'java standard formatting' over 'eclipse built-in formatting'.

@sing1ee
sing1ee commented Aug 15, 2012

Thanks,that's my problem。As you said, I use eclipse.I will do this without
changing the format。
在 2012-8-16 上午6:42,"Jonathan Leibiusky" notifications@github.com写道:

Can you please resend the pull request without all the formatting changes?
It is very very hard to follow the changes, as you see like everything has
changed.
If using eclipse, make sure you have set 'java standard formatting' over
'eclipse built-in formatting'.


Reply to this email directly or view it on GitHubhttps://github.com/xetorthio/jedis/pull/331#issuecomment-7771820.

@sing1ee
sing1ee commented Aug 17, 2012

hi, is there any convient method to keep the format right?

And one more problem, When I use the pipeline with jedis, I found that he
lpush and rpush method can't accept the multiple element, but just one
element at a time. Is there any special concideration.

2012/8/16 张成 zh.milo@gmail.com

Thanks,that's my problem。As you said, I use eclipse.I will do this without
changing the format。
在 2012-8-16 上午6:42,"Jonathan Leibiusky" notifications@github.com写道:

Can you please resend the pull request without all the formatting changes?

It is very very hard to follow the changes, as you see like everything has
changed.
If using eclipse, make sure you have set 'java standard formatting' over
'eclipse built-in formatting'.


Reply to this email directly or view it on GitHubhttps://github.com/xetorthio/jedis/pull/331#issuecomment-7771820.

@xetorthio
Owner

What do you mean by keeping the format right?

It seems like lpush and rpush variadic overloads are missing.
Would you mind opening an issue in github about this?

thanks!

On Fri, Aug 17, 2012 at 2:32 AM, sing1ee notifications@github.com wrote:

hi, is there any convient method to keep the format right?

And one more problem, When I use the pipeline with jedis, I found that he
lpush and rpush method can't accept the multiple element, but just one
element at a time. Is there any special concideration.

2012/8/16 张成 zh.milo@gmail.com

Thanks,that's my problem。As you said, I use eclipse.I will do this
without
changing the format。
在 2012-8-16 上午6:42,"Jonathan Leibiusky" notifications@github.com写道:

Can you please resend the pull request without all the formatting
changes?

It is very very hard to follow the changes, as you see like everything
has
changed.
If using eclipse, make sure you have set 'java standard formatting'
over
'eclipse built-in formatting'.


Reply to this email directly or view it on GitHub<
https://github.com/xetorthio/jedis/pull/331#issuecomment-7771820>.


Reply to this email directly or view it on GitHubhttps://github.com/xetorthio/jedis/pull/331#issuecomment-7808838.

@veselov
Contributor
veselov commented Mar 1, 2013

I would have to disagree that using Map of anything is proper for this operation. You are effectively using map as simple carrier of pair of values (value and score). By JDK contract, Map is a unique association of a value to a key. Though it's true that if the Map key/value is reversed to <Value, Score>, and considering that ZSET values must be unique, it may make logical sense, but it creates a problem of populating such map. May be Map<Value, Score> is OK to have, but then an alternative method of specifying these tuples should be provided.

For the application to pass down a Map, the application will need to populate such Map. Map.put() method is inherently a slow performing method, because it has to look the value up before adding it to the map. Even for a hash Map, where look up is nearly constant, it's still slower than if the application could just chain up the data that needs to be sent out to Redis. Also, the internal implementation of Jedis is such that a binary call is performed. In case of binary data, the Map will have to be Map<byte[], Double>, and having byte[] values as keys is really not a good idea.

May be the arguments can be two separate arrays (of values and scores), or an array/collection of Map.Entry<>, or something to this nature.

@xetorthio
Owner

closed by #515

@xetorthio xetorthio closed this Jan 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment