Skip to content

Commit

Permalink
Version 3 proposal: Store optional member data in a single hash
Browse files Browse the repository at this point in the history 
This has the benefit of not using a separate hash for every single
member's data. As is updated in the documentation, you can store
more data for a given member by, for example, encoding a Hash
in JSON. Not only will this save on Hash-splosion if using member
data in a leaderboard, it also means that when deleting a
leaderboard, we can also delete ALL of the member data at once. In
version 2.x, if using member data, you would have to go through and
delete the member data hashes individually. Yikes!
  • Loading branch information
David Czarnecki committed Sep 16, 2012
1 parent 9374a01 commit db1776e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 33 deletions.
16 changes: 10 additions & 6 deletions README.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,33 @@ Get some information about your leaderboard:
=> 1
```

The `rank_member` call will also accept an optional hash of member data that could
The `rank_member` call will also accept an optional string member data that could
be used to store other information about a given member in the leaderboard. This
may be useful in situations where you are storing member IDs in the leaderboard and
you want to be able to store a member name for display. Example:
you want to be able to store a member name for display. You could use JSON to
encode a Hash of member data. Example:

```ruby
highscore_lb.rank_member('84849292', 1, {'username' => 'member_name'})
require 'json'
highscore_lb.rank_member('84849292', 1, JSON.generate({'username' => 'member_name'})
```

You can retrieve, update and remove the optional member data using the
`member_data_for`, `update_member_data` and `remove_member_data` calls. Example:

```ruby
highscore_lb.member_data_for('84849292')
JSON.parse(highscore_lb.member_data_for('84849292'))
=> {"username"=>"member_name"}

highscore_lb.update_member_data('84849292', {'last_updated' => Time.now, 'username' => 'updated_member_name'})
highscore_lb.update_member_data('84849292', JSON.generate({'last_updated' => Time.now, 'username' => 'updated_member_name'}))
=> "OK"
highscore_lb.member_data_for('84849292')
JSON.parse(highscore_lb.member_data_for('84849292'))
=> {"username"=>"updated_member_name", "last_updated"=>"2012-06-09 09:11:06 -0400"}

highscore_lb.remove_member_data('84849292')
```

If you delete the leaderboard, ALL of the member data is deleted as well.

Get some information about a specific member(s) in the leaderboard:

Expand Down
24 changes: 12 additions & 12 deletions lib/leaderboard.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ def delete_leaderboard
#
# @param leaderboard_name [String] Name of the leaderboard.
def delete_leaderboard_named(leaderboard_name)
@redis_connection.del(leaderboard_name)
@redis_connection.multi do |transaction|
transaction.del(leaderboard_name)
transaction.del(member_data_key(leaderboard_name))
end
end

# Rank a member in the leaderboard.
Expand All @@ -124,9 +127,7 @@ def rank_member(member, score, member_data = nil)
def rank_member_in(leaderboard_name, member, score, member_data)
@redis_connection.multi do |transaction|
transaction.zadd(leaderboard_name, score, member)
if member_data
transaction.hmset(member_data_key(leaderboard_name, member), *member_data.to_a.flatten)
end
transaction.hset(member_data_key(leaderboard_name), member, member_data) if member_data
end
end

Expand All @@ -146,7 +147,7 @@ def member_data_for(member)
#
# @return Hash of optional member data.
def member_data_for_in(leaderboard_name, member)
@redis_connection.hgetall(member_data_key(leaderboard_name, member))
@redis_connection.hget(member_data_key(leaderboard_name), member)
end

# Update the optional member data for a given member in the leaderboard.
Expand All @@ -163,7 +164,7 @@ def update_member_data(member, member_data)
# @param member [String] Member name.
# @param member_data [Hash] Optional member data.
def update_member_data_in(leaderboard_name, member, member_data)
@redis_connection.hmset(member_data_key(leaderboard_name, member), *member_data.to_a.flatten)
@redis_connection.hset(member_data_key(leaderboard_name), member, member_data)
end

# Remove the optional member data for a given member in the leaderboard.
Expand All @@ -178,7 +179,7 @@ def remove_member_data(member)
# @param leaderboard_name [String] Name of the leaderboard.
# @param member [String] Member name.
def remove_member_data_in(leaderboard_name, member)
@redis_connection.del(member_data_key(leaderboard_name, member))
@redis_connection.hdel(member_data_key(leaderboard_name), member)
end

# Rank an array of members in the leaderboard.
Expand Down Expand Up @@ -218,7 +219,7 @@ def remove_member(member)
def remove_member_from(leaderboard_name, member)
@redis_connection.multi do |transaction|
transaction.zrem(leaderboard_name, member)
transaction.del(member_data_key(leaderboard_name, member))
transaction.del(member_data_key(leaderboard_name), member)
end
end

Expand Down Expand Up @@ -815,11 +816,10 @@ def intersect_leaderboards(destination, keys, options = {:aggregate => :sum})
# Key for retrieving optional member data.
#
# @param leaderboard_name [String] Name of the leaderboard.
# @param member [String] Member name.
#
# @return a key in the form of +leaderboard_name:data:member+
def member_data_key(leaderboard_name, member)
"#{leaderboard_name}:member_data:#{member}"
# @return a key in the form of +leaderboard_name:member_data+
def member_data_key(leaderboard_name)
"#{leaderboard_name}:member_data"
end

# Validate and return the page size. Returns the +DEFAULT_PAGE_SIZE+ if the page size is less than 1.
Expand Down
26 changes: 14 additions & 12 deletions spec/leaderboard_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
rank_members_in_leaderboard

@redis_connection.exists('name').should be_true
@redis_connection.exists('name:member_data').should be_true
@leaderboard.delete_leaderboard
@redis_connection.exists('name').should be_false
@redis_connection.exists('name:member_data').should be_false
end

it 'should allow you to pass in an existing redis connection in the initializer' do
Expand Down Expand Up @@ -170,10 +172,10 @@

members = @leaderboard.members_from_score_range(10, 15, {:with_scores => true, :with_rank => true, :with_member_data => true})

member_15 = {:member => 'member_15', :rank => 11, :score => 15.0, :member_data => {'member_name' => 'Leaderboard member 15'}}
member_15 = {:member => 'member_15', :rank => 11, :score => 15.0, :member_data => {:member_name => 'Leaderboard member 15'}.to_s}
members[0].should == member_15

member_10 = {:member => 'member_10', :rank => 16, :score => 10.0, :member_data => {'member_name' => 'Leaderboard member 10'}}
member_10 = {:member => 'member_10', :rank => 16, :score => 10.0, :member_data => {:member_name => 'Leaderboard member 10'}.to_s}
members[5].should == member_10
end

Expand All @@ -196,34 +198,34 @@
@leaderboard.total_members.should be(Leaderboard::DEFAULT_PAGE_SIZE)
leaders = @leaderboard.leaders(1, {:with_scores => false, :with_rank => false, :with_member_data => true})

member_25 = {:member => 'member_25', :member_data => { "member_name" => "Leaderboard member 25" }}
member_25 = {:member => 'member_25', :member_data => { :member_name => "Leaderboard member 25" }.to_s }
leaders[0].should == member_25

member_1 = {:member => 'member_1', :member_data => { "member_name" => "Leaderboard member 1" }}
member_1 = {:member => 'member_1', :member_data => { :member_name => "Leaderboard member 1" }.to_s }
leaders[24].should == member_1
end

it 'should allow you to retrieve optional member data' do
@leaderboard.rank_member('member_id', 1, {'username' => 'member_name', 'other_data_key' => 'other_data_value'})

@leaderboard.member_data_for('unknown_member').should == {}
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name', 'other_data_key' => 'other_data_value'}
@leaderboard.member_data_for('unknown_member').should be_nil
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name', 'other_data_key' => 'other_data_value'}.to_s
end

it 'should allow you to update optional member data' do
@leaderboard.rank_member('member_id', 1, {'username' => 'member_name'})

@leaderboard.member_data_for('member_id').should == {'username' => 'member_name'}
@leaderboard.update_member_data('member_id', {'other_data_key' => 'other_data_value'})
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name', 'other_data_key' => 'other_data_value'}
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name'}.to_s
@leaderboard.update_member_data('member_id', {'username' => 'member_name', 'other_data_key' => 'other_data_value'})
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name', 'other_data_key' => 'other_data_value'}.to_s
end

it 'should allow you to remove optional member data' do
@leaderboard.rank_member('member_id', 1, {'username' => 'member_name'})

@leaderboard.member_data_for('member_id').should == {'username' => 'member_name'}
@leaderboard.member_data_for('member_id').should == {'username' => 'member_name'}.to_s
@leaderboard.remove_member_data('member_id')
@leaderboard.member_data_for('member_id').should == {}
@leaderboard.member_data_for('member_id').should be_nil
end

it 'should allow you to call leaders with various options that respect the defaults for the options not passed in' do
Expand Down Expand Up @@ -268,7 +270,7 @@
@leaderboard.member_at(26)[:rank].should == 26
@leaderboard.member_at(50)[:rank].should == 50
@leaderboard.member_at(51).should be_nil
@leaderboard.member_at(1, :with_member_data => true)[:member_data].should == {'member_name' => 'Leaderboard member 50'}
@leaderboard.member_at(1, :with_member_data => true)[:member_data].should == {:member_name => 'Leaderboard member 50'}.to_s
@leaderboard.member_at(1, :use_zero_index_for_rank => true)[:rank].should == 0
end

Expand Down
6 changes: 3 additions & 3 deletions spec/reverse_leaderboard_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@

members = @leaderboard.members_from_score_range(10, 15, {:with_scores => true, :with_rank => true, :with_member_data => true})

member_10 = {:member => 'member_10', :rank => 10, :score => 10.0, :member_data => {'member_name' => 'Leaderboard member 10'}}
member_10 = {:member => 'member_10', :rank => 10, :score => 10.0, :member_data => {:member_name => 'Leaderboard member 10'}.to_s}
members[0].should == member_10

member_15 = {:member => 'member_15', :rank => 15, :score => 15.0, :member_data => {'member_name' => 'Leaderboard member 15'}}
member_15 = {:member => 'member_15', :rank => 15, :score => 15.0, :member_data => {:member_name => 'Leaderboard member 15'}.to_s}
members[5].should == member_15
end

Expand Down Expand Up @@ -122,7 +122,7 @@
@leaderboard.member_at(26)[:rank].should == 26
@leaderboard.member_at(50)[:rank].should == 50
@leaderboard.member_at(51).should be_nil
@leaderboard.member_at(1, :with_member_data => true)[:member_data].should == {'member_name' => 'Leaderboard member 1'}
@leaderboard.member_at(1, :with_member_data => true)[:member_data].should == {:member_name => 'Leaderboard member 1'}.to_s
@leaderboard.member_at(1, :use_zero_index_for_rank => true)[:rank].should == 0
end

Expand Down

2 comments on commit db1776e

@czarneckid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgadbois Any thoughts on this proposal for version 3 of the gem? I neglected at the time to consider the implications of the large # of hashes and having to cleanup member data in an easy way when you remove a leaderboard.

@jgadbois
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I didn't anticipate that either. I like passing a hash as the member data and having leaderboard conver it internally vs having to pass in a JSON string. Are you just trying to allow more flexibility in the type of member data?

Please sign in to comment.