Mailing-List: contact jackrabbit-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jackrabbit-dev@incubator.apache.org
Received-SPF: pass (asf.osuosl.org: domain of ppurang@gmail.com designates
 66.249.82.200 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references;
        b=FuTk8tr19n0je6cCPskCvK4wbmVCu2cEVj8c/5kVph5FEhjfB5wFBKIOHPK4HFB5rpo7B2XpdNkFqFyQKYOMqxr46ni6IdR3XmfgnZU4ZlvD5hphJ4F1D9aCu82jq8zNdQAWW9wrXM04/3LV1DBDyQv+rn/usq7YWkMRx4o9h0I=
Message-ID: <ef62a9ce0602221338ic2e5135v4e5731d1f51785e3@mail.gmail.com>
Date: Wed, 22 Feb 2006 22:38:31 +0100
From: "Piyush Purang" <ppurang@gmail.com>
To: jackrabbit-dev@incubator.apache.org
Subject: Re: use of java 1.4 assert
In-Reply-To: <43F5E658.40908@gmx.net>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="----=_Part_4414_32435372.1140644311205"
References: <43F46783.7030405@gmx.net> <43F5E658.40908@gmx.net>

------=_Part_4414_32435372.1140644311205
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Hi Marcel,

I finally had time to look into your usage of assert and I can't convince
myself that it is correct usage.

The reason you decided to use an assert-statement is that on a production
environment (PE) asserts will be turned off (which is a very likely
scenario) and hence performance will be better as per the promise [1].

The code was

 if (!listeners.containsKey(listener)) {
            listeners.put(listener, listener);
and now is

synchronized (listeners) {
            assert (!listeners.contains(listener));
            listeners.add (listener);

The previous version silently ignored requests to re-register listeners tha=
t
are already registered. That is the client doesn't have to do any
bookkeeping he can register umpteen times and he will be delivered the even=
t
just once.

Now in the new version on the developer's box it will throw an
AssertionError and the developer would have been strong-muscled into
following the dictum register just once. Most of the times though Listeners
that would be registering themselves don't want to hassle theme selves in
keeping tabs on how many times did they register. (At least I never
programmed my listeners like that. I just registered.) Hence there is a
possibility that listeners on the PE get added multiple-times to the
listener queue and receive the same event many times unless we again do som=
e
bookkeeping either on our side or listeners do it themselves (Performance
will be hit). And no listener expects to be told about the same event more
than once; that is something we all can agree on.

And if you still want to call it a pre-condition then [1] clearly points ou=
t
don't use Assertions to implement pre-conditions for public methods.

Do you agree with my analysis? I of course don't have the kind of overview
that you have of jackrabbit. Maybe my presumption that the outside world
will be registering listeners is false. Even in that case I am not 100%
convinced about using assertions to fit the role.

How about using a weak hash set?

Cheers
Piyush

[1] http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html

 On 2/17/06, Marcel Reutegger <marcel.reutegger@gmx.net> wrote:
>
> I've just committed a change that uses asserts in the sources.
>
> Users of IntelliJ have to manually change the language level of the
> jackrabbit project to 1.4. It seems that the idea maven plugin does not
> take the maven.compile.source=3D1.4 property into account when generating
> a project file :-/
>
> regards
>   marcel
>
> Marcel Reutegger wrote:
> > hi all,
> >
> > I'm currently working on an improvement regarding memory consumption in
> > jackrabbit (replace reference map for listener in item states with a
> > more light weight collection class). doing that, I would like to use th=
e
> > java 1.4 assert facility to include preconditions that make sure a
> > listener is only registered once. that way I don't have to check every
> > time if a listener is already registered.
> >
> > using 1.4 asserts would required the '-source 1.4' switch when compilin=
g
> > jackrabbit. our maven project.properties already adds that switch. so,
> > there would be nothing to do on our side. but probably a comment in
> > documentation that talks about this.
> >
> > per default asserts are disabled and will not impact performance.
> > when running the unit tests, asserts would then be enabled using the -e=
a
> > switch for the test run.
> >
> > what's the general feeling about this?
> >
> > regards
> >  marcel
> >
>
>

------=_Part_4414_32435372.1140644311205--