« Return to Thread: NSSound Reimplementation

Re: NSSound Reimplementation

by Richard Frith-Macdonald-2 :: Rate this Message:

On 19 Jun 2009, at 08:05, Fred Kiefer wrote:

> Stefan Bidigaray wrote:
>> It would really help if I attached the file...
>>
>
> I don't understand the general concept here, so just a few detail
> comments on the code itself.
>
> Hiding the implementation details in the header is a good thing, but
> it
> makes the code in the implementation harder to read. This could be
> improved by using local variables (or even Macros). For example
>
>
> - (BOOL) pause
> {
> NSConditionLock *lock = (NSConditionLock*)_private[2];
>
> if ([lock condition] == SOUND_SHOULD_PAUSE)
> {
> return NO;
> }
> if ([lock tryLock] == NO)
> {
> return NO;
> }
> [lock unlockWithCondition: SOUND_SHOULD_PAUSE];
> return YES;
> }

I would go further and ask what the point of the _private array is?

By convention all the instance variables are private (that's what the
leading underscore in the variable name means), so if all that's
required is to inform developers that they should not use the ivars
directly, there's no point in having the array, but maybe it would be
worth using a @private declaration.

On the other hand, if the idea is to hide the implementation details
so that ivar layouts won't need to change with future revisions
(something I think we should all be doing), my preference would be to
have a single pointer to a structure containing the required data.
eg.

struct _NSSoundInternal;

@interface NSSound : NSObject <NSCoding, NSCopying>
{
@private
struct _NSSoundInternal *_internal;
}

Then the designated initialiser allocates memory for the structure,
clears it, and assigns the pointer when an instance is initialised,
and -dealloc frees the memory at the end, and all internal references
to ivars just go indirect from that pointer. The implementation is
completely hidden/private and can be changed in subsequent releases
without breaking the ABI.

_______________________________________________
Gnustep-dev mailing list
Gnustep-dev@...
http://lists.gnu.org/mailman/listinfo/gnustep-dev

« Return to Thread: NSSound Reimplementation