Thanks for reviewing this, replies inline.
On 18/08/2012 13:56, Dmitry Samersoff wrote:
Alan,
1. 315 - IMHO, it's better to call checkAccess() before null pointer
check. This problem exists in original code as well.
If there are two or more exceptions that are appropriate for a
particular case then developers have to prepared for either. In the case
if someone invokes the addPropertyChangeListener with null and at the
same time would fail the permission check then either exception is
valid; you can't assume one or the other. There can of course be cases
where you have to be careful to do a permission check before other
checks because failing some other check might reveal something to an
adversary. This isn't the case here and in addition I don't think we
should change existing behavior without good reason.
2. This place is not clean for me - env is constant under loop.
is it intentional?
975 for (int i = 0; i< count; i++) {
976 listener.propertyChange(ev);
977 }
It is intentional as the spec requires that a listener be invoked for as
many times as it is registered. I could of course have used a while loop
instead, maybe "while (count-- > 0)" but I think the above is clear.
-Alan
