Re: Setter Gettor method style

[Pascal Bourguignon]

Oops, sorry, previous copy sent prematurely...

> From: Alexander Malmberg <alexander@malmberg.org>
> Date: Sun, 04 Aug 2002 19:25:24 +0200
>
> > The rules I got along with the Foundation in NeXTSTEP 3.3 were:
> >
> >     - initXxxx return retained objects, (POST: [result retainCount]==1)
>
> Or nil and the original object has been deallocated.
>
> >     - classXxxx (stringWithFormat:, etc) return autoreleased objects,
> >
> >     - accessors, since they return attributes don't return
> >       autoreleased objects, but retained objects
> >       (POST: [result retainCount]>=1).
>
> I'd add:
>
> - non-accessors (like [NSString -lowercaseString]) return autoreleased
> objects unless the name is clearly marked (eg. getFoo: (id *)f or
> [NSCell -_nonAutoreleasedTypingAttributes]).

Agree.

> This is acceptable for performance, and it seems most of the existing
> code works this way, but it leads to objects with very uncertain
> lifetimes, especially wrt threads, instead of the clear 'it's in the
> current autoreleasepool'.
>
> [snip]
> > There's  no reason to  retain the  title.  That  is, one  expects, and
> > should check, that the preconditions/postconditions of setColor: don't
> > have any side-effect on the title object returned before.
>
> But this means that you'd have to check for not-necessarily-obvious
> side-effects for just about every method when you're using accessors.
> The semantics of accessors would be very non-obvious. If you retain
> everything just to be safe, the accessors might as well retain it for
> you.
>
> [snip]
> > But  as always,  my  main point  is  that this  should be  explicitely
> > documented.   The use  of return([[attribute  retain]autorelease]); is
> > not failproof (foolproof?!);
> >
> >       NSAutoreleasePool* pool=[[NSAutoreleasePool alloc]init];
> >       NSString* title=[stuff title];
> >       [stuff setTitle:@"New title"];
> >       [pool release];
> >       NSLog(@"old title was %@\n",title); // title is deleted...
> >
> [snip long example]
>
> There is no problem here. The accessor returns an autoreleased object.
> If you want to bring the object out from an autoreleasepool, you'll have
> to retain it and autorelease it again in the next autoreleasepool, just
> as you'd have to do with any other autoreleased object.
>
> > The  rule that  says that  you  must retain  the objects  you need  is
> > failproof in all cases.
>
> No. Obscure cases involving multiple threads could be constructed where
> {[foo retain] autorelease] worked properly but retaining it after it's
> been returned doesn't.


I find it difficult to choose one way or another.

The point is that we're speaking of attributes:

    +---------------------------------+
    |           SampleClass           |
    +---------------------------------+
    | attribute:string;               |
    |                                 |
    +---------------------------------+
    | setAttribute(newValue:string);  |
    | getAttribute() return string;   |
    |                                 |
    +---------------------------------+
                        Diagram 1.

that are actually implemented with another object and a relation:

    +-----------------------------------+             attribute +----------+
    |           SampleClass             |-----------------------| NSString |
    +-----------------------------------+ 0,*                 1 +----------+
    |                                   |                            |
    +-----------------------------------+                           /_\
    | setAttribute(newValue:NSString*); |                            |
    | getAttribute() return NSString*;  |                 +-----------------+
    +-----------------------------------+                 | NSMutableString |
                                                          +-----------------+
                        Diagram 2.


I feel that the right solution is to take copies and return copies:

  -(void)setAttribute:(NSString*)newValue
  {
     [_attribute release];
     _attribute=[[newValue copy/*or mutableCopy if it can be edite by self*/]
                      retain];
  }//setAttribute:;

  -(NSString*)getAttribute
  {
    return([_attribute copy]);
  }//getAttribute;


(Of course, a valid option in the settor is to have a mutable object and modify 
it:
  -(void)setAttribute:(NSString*)newValue
  {
     [_attribute setStringValue:newValue];
  }//setAttribute:;
)


Well, since getAttribute returns a copy for the exclusive usage of the
caller, it could as well return a mutableCopy to allow the called edit
the result without having to do a new mutable copy itself.

What is the difference in performance of copy and mutableCopy?


Now, the problem comes when  one realize that most often setAttribute:
is passed an newly allocated / constructed autoreleased NSString, that
would not be further owned and used by the called.

Then, as  an optimisation,  we want to  use directly this  object, and
instead  of a  mere "attribute",  incorporated to  the object,  we are
actually establishing a relation with a NSString object.

  -(void)setAttribute:(NSString*)newObject
  {
     [_attribute release];
     _attribute=[newObject retain];
  }//setAttribute:;

With the  problem that if newObject  is a mutable object,  it could be
edited by the caller without  self object noticing.  Which would be ok
if attribute were really a  distinct object in relation with self, and
not an internal attribute.

So, if you are in the case  of Diagram 1 and want to have attribute, I
would say that you  must copy values. And only if you  are in the case
of  Diagram 2  and  have  relationship with  NSString  and other  data
objects you  may keep a pointer to  the same object and  then you must
expect it to be edited by the rest of the program.


Now, about the gettor.

If  you consider you  have an  attribute (Diagram  1) then  you should
return  a copy  object,  and perhaps,  as  a favor  to  the caller,  a
mutableCopy.

Returning  a  pointer  to  one  of  your  attribute  is  a  breach  of
encapsulation. 


If you consider  you have a relationship with  a value object (Diagram
2),  then  of  course you  return  the  object  with which  you're  in
relationship with:


  -(NSString*)getAttribute
  {
     return(_attribute);
  }

and there  is no reason  to do anything  further.  Of course,  in that
case, you must accept that the returned object may be edited (it could
be a mutable object after all).



>From the point of view of the caller, 

       NSString* title=[stuff title];
       NSLog(@"title= %@\n",title);
       [stuff setTitle:@"New title"];
       NSLog(@"title= %@\n",title);

without further information you could as well expect to get as output:

        title= Old title
        title= New title

What  I mean  here  is  that if  you  don't consider  title  to be  an
attribute, and  if the  -title method  does not return  a copy  of the
value, then  I find as valid  to have this kind  of implementation and
behavior:

   -(void)setTitle:(NSString*)newValue
   {
      [_title/*a NSMutableString!*/ setString:newValue];
   }//setTitle:;

   -(NSString*)title
   {
      return(_title);
   }//title;

and  once again  in  this case,  I don't  see  any need  for retain  /
autorelease games.



Therefore,  we  are  either:

   - in a  case where  we have attributes,  encapsulated in  the owner
     object, and in this case we  keep a copy in the settor and return
     a copy in the gettor,

     hence  the  caller has  no  worry  about  the returned  attribute
     values: they belong to it.


   - or  in a  case where  we have  relationships with  external value
     objects, and  in this case the  settor retains it  and the gettor
     just returns the reference to the value object.
     
     and here we have the problem in multithreaded programs, where the
     target of  the relationship may  be released after the  return of
     the "gettor" and before the caller has time to retain it.

This is not  a problem specific (I would  say, related) to attributes,
but  to   relationships  with  dependant   objects  in  the   case  of
multithreading.


The OpenStepSpecification says, for example, for NSWindow:

 - (void) setTitle:(NSString *) aString
    Makes  aString  the window's title

 - (NSString *) title
    Returns the window's title string. 


Strangely  enough,  in the  GNUstep  implementation,  the  title of  a
NSWindow is implemented as a relationship with the value object (which
means that if a NSMutableString is passed to -[NSWindow setTitle:], it
could  be modified thereafter,  and -[NSWindow  title] would  return a
reference to the same, modified, object, with a value not reflected in
the  title  bar of  the  window!  But  at  the  same time,  -[NSWindow
setTitle:] invokes -[NSMiniWindowView setTitle:] which correctly takes
a copy  of the value  (title is considered  to be an attribute  of the
mini window) with: [titleCell setStringValue: aString].


I  would move to  have a  GNUstep Documetation  say, for  example, for
NSWindow:

 The title is an attribute of the NSWindow:

 - (void) setTitle:(NSString *) aString
   Takes a copy of aString as the title of the window.
   Update the window to display this new title.

 - (NSString *) title
   Returns a copy of the last title set to the window.


However, in the case of the contentView, for example, where we have as
OpenStepSpecifications:

 - (id) contentView
    Returns the NSWindow's content view. 
 
 - (void) setContentView: (NSView *) aView
    Makes  aView  the NSWindow's content view. 

in the GNUstep implementation, we have the settor which basically does:

   [_contentView release];_contentView=[newContentView retain];

and the gettor:

   - (id) contentView
   {
      return _contentView;
   }

which clearly  is not  thread-clean: one thread  could setContentView:
while the other does contentView and  get a released object (we end up
into -[GSMutableArray removeObjectAtIndex:] which sends release to the
removed object!).

In those cases, a return([_contentView retain]autorelease]); would be
useful, and the GNUstep Documentation could say:

 A relation between a NSWindow and its contentView is established with
 setContentView: and traversed with contentView:

 - (id) contentView

    Returns the content  view of the NSWindow at the  time of the call
    (it could have  been changed since in another  thread). The result
    is retain-autoreleased.
 
 - (void) setContentView: (NSView *) aView

    Breaks  the  relationship  between   this  NSWindow  and  its  old
    contentView, and establish  the relationship between this NSWindow
    and  aView  as  its  new  contentView.   The  old  contentView  is
    released, aView is  retained. If aView is nil,  a new empty NSView
    is instanciated to be used instead.


--
__Pascal_Bourguignon__                   http://www.informatimago.com/
----------------------------------------------------------------------