gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: problems compiling NSAnimation.m


From: Richard Frith-Macdonald
Subject: Re: problems compiling NSAnimation.m
Date: Thu, 10 May 2007 15:45:51 +0100


On 10 May 2007, at 15:28, Xavier Glattard wrote:

I fixed this to make __gs_isLocked an ivar rather than declaring it
locally (which was pretty suboptimal).  I also fixed a bug in the
unlock macro (it was setting the lag to the wrong value), and added
assertions to check that the macro is not misused.

Bad. If __gs_locked is needed (and as i said i think it is not : i
made
an error) then it must be declared in all methods. If __gs_isLocked is an ivar, its value can be changed in another method then the [unlock]
will never be called.

Sure, but similarly if it's a local variable its value can be
incorrectly changed within a method or function where it is declared.

If it is then you made an other programming error... ;-)
Its name is prefixed by an underscore (and by 'gs'!) so it shouldn't
be used by anyone.
Moreover an ivar is more easier to be incorrectly changed...
in an other method or even in a child class!

You are missing my point ... the variable is not used by any other code!
You can't prevent it being used if someone wants to introduce a bug, but there is no reason why anyone should do that.
Making it an ivar *fixes* the code to work with an older compiler

An ivar already exists: _isThreaded.

But that doesn't record the same thing.  The __gs_locked boolean
records whether the lock is locked, not whether the animation is
threaded.

I agree : _gs_isLocked records whether the _local_ lock is locked.
So it has to be a _local_ variable.
_gs_isLocked as an ivar is an error.

Actually that's not correct ... the lock is an ivar and is *not* local to the method, so as the lock can be modified by any method, the variable to record its state similarly needs to be modified in any method where the lock is modified.

(...) I only looked at the code enough to
fix it to compile and manage locking correctly
(...)

Did you run GSTest ?

Nope.  Does that now test animations?






reply via email to

[Prev in Thread] Current Thread [Next in Thread]