On Wed, 2008-07-02 at 09:44 +0100, Martin Ritchie wrote:
> 2008/7/1 Gordon Sim <[EMAIL PROTECTED]>:
> > In the notifyMessage(UnprocessedMessage):void method of BasicMessageConsumer
> > there are still some commented out lines from a very old commit (merged to
> > trunk over a year ago).
> >
> > Am I right in assuming there is no real reason for this being commented out
> > and that it can therefore be deleted?
> >
> > I would just delete it quietly, but as I don't have the context for the
> > change that required disabling these lines of code, I thought a quick mail
> > might be in order (in case they were serving as some sort of reminder or
> > something?!).
> >
> > I'm not picking on java, honest! I'm sure there are similar things in e.g.
> > the c++ code. I'm pointing this case out because it just came up in
> > conversation. I'd be as happy for people to point out issues they notice on
> > any code I tend to work on; once noticed its easy to clean up.
> >
> > The biggest chunk of code in question looks like:
> >
> > // synchronized (_closed)
> > {
> > // if (!_closed.get())
> > {
> >
> > //preDeliver(jmsMessage);
> >
> > notifyMessage(jmsMessage);
> > }
> > // else
> > // {
> > // _logger.error("MESSAGE REJECTING!");
> > // _session.rejectMessage(jmsMessage, true);
> > // //_logger.error("MESSAGE JUST DROPPED!");
> > // }
> >
> > but the '// synchronized (_closed)' part is replicated in several places.
>
> I'm happy with the removal. IIRC the synchronization bits were just
> commented out as the client is in such a poor state wrt
> threading/locking that we were not 100% sure of the changes.
>
> However, there is no reason to be checking in commented out code (even
> if it is commented) it is just too fragile and for me just says the is
> doubt about the correctness of the change.
>
> My recent commit to the performance change is a prime example. I don't
> like the commenting out of the code but we need to decide what we are
> going to do with the broken code.. spend the time on it to fix it and
> use it or just leave it. I think just now it will be left as no one
> has time to work on it .. hence the comment and the commenting out of
> code. I was very surprised not to get a good flaming for it though.
>
> Cheers
>
> Martin
