Re: Encapsulates users and subscriptions management into WaveletInfo.

[veg...@gmail.com]

Pushed as 16c3d22cf118

http://codereview.waveprotocol.org/614002/diff/5001/6003
File src/org/waveprotocol/box/server/frontend/WaveletInfo.java (right):

http://codereview.waveprotocol.org/614002/diff/5001/6003#newcode58
Line 58: this.explicitParticipants = Sets.<ParticipantId> newHashSet();
On 2011/09/02 15:58:19, Soren Lassen wrote:
> I believe <ParticipantId> is redundant.

Done.

http://codereview.waveprotocol.org/614002/diff/5001/6003#newcode90
Line 90: final MapMaker mapMaker = new MapMaker();
On 2011/09/02 15:58:19, Soren Lassen wrote:
> normally, you just create a new MapMaker instance whenever you create
a
> computing map

> new MapMaker().makeComputingMap(....);

> I don't think there are advantages to sharing the MapMaker instance
but, on the
> other hand, what you have should work

Done.

http://codereview.waveprotocol.org/614002/diff/5001/6003#newcode183
Line 183: version = waveletInfo.getCurrentVersion();
On 2011/09/02 15:58:19, Soren Lassen wrote:
> FWIW, it will be fewer lines of code to just say

> synchronize (waveletInfo) {
> return waveletInfo.getCurrentVersion();
> }

> same in lines 197, 214 below, but what you have works fine, too

Done.

http://codereview.waveprotocol.org/614002/diff/5001/6003#newcode197
Line 197:
Collections.unmodifiableSet(Sets.newHashSet(waveletInfo.explicitParticipants));
On 2011/09/02 15:58:19, Soren Lassen wrote:
> it will be more direct to use ImmutableSet.copyOf

> same in line 214 below

Done.

http://codereview.waveprotocol.org/614002