Re: NSSound Reimplementation

[Richard Frith-Macdonald]

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.