Re: NSButtonCell value, contents and cell type implementation rewritten

[Fred Kiefer]

Quentin Mathé wrote:
> Le 1 sept. 04, à 23:30, Fred Kiefer a écrit :
>
> > Quentin Mathé wrote:
> >
> > > Le 30 août 04, à 21:06, Fred Kiefer a écrit :
> ok, sorry for my laziness, but well this rule doesn't exist… :-)
>
>  From GNUstep coding guidelines :
> "Finally, my own preference (not part of the standard in any way) is to 
> generally use curly brackets for control constructs, event where only 
> one line of code is involved"
>
> Moreover "this rule" is poorly (to not said never) respected in GNUstep, 
> there is lot of work to do if you want GNUstep respects such rule.
>
> Well we can discuss it… why not…

I was only wondering as you did change a line which followed the 
non-rule into one which didn't.

> > > > The only question I have with this patch is, if it would not be 
> > > > simpler to use a call to the setType: method in the class NSCell, 
> > > > instead of the  direct setting of _cell.type and just overwrite that 
> > > > method in NSButtonCell to do nothing.
> > >
> > > Well it was my first idea, but it isn't possible.
> >
> >
> > OK, but why not? A bit of explanation might help me see the light.
>
>
> Sorry again, I didn't understand what you was meaning…
> My patch isn't in any way a NSCell rewrite, it is a NSButtonCell 
> rewrite, in the case you want to rewrite NSCell to use -setType: more 
> often you can. For NSButtonCell, the only time the cell type is changed, 
> it is with -setImagePosition: and in this precise case it is impossible 
> to use the NSCell method -setType because of the side effect.  Otherwise 
> it is needed to override -setType: to do nothing in NSButtonCell to 
> match somewhat the Cocoa behavior, and more importantly to have a 
> consistent and nice behavior (I explained that in detail and it seems we 
> have already agreed on this in the previous thread  [PATCH] Proposal to 
> fix NSButtonCell issue with -setFont: -setTitle:)

You already do need to change a few methods on NSCell and even more in 
NSButtonCell just because NSCell doesn't do what you want it to do.
Your patch consists of two main area. One is about exchanging the way 
state and title are handled, which is mostly fine for me (apart from the 
cleanup that Alexander wants for NSButton), the other part is the way 
the type of a NSBUttonCell changes. For this bit my impression is that 
your patch would be simpler and cleaner if we changed NSCell to use 
setType: everywhere (apart from the init method) and than would just 
overwrite this method in NSButtonCell.

> > > > Could you please test if Cocoa does call the method or if everything 
> > > > is done on the same low level as we do it.
> > >
> > > The Cocoa behavior is explained in the patch :
> > > +    * On Mac OS X, the NSButtonCell cell type is NSTextCellType by 
> > > default or
> > > +    * NSImageCellType if the initialization has been done with 
> > > -initImageCell:,
> > > +    * it should be noted that the cell type never changes later.
> >
> >
> > Thats exaclty what I thought in the init method this method is not 
> > called, but I would expect it to be called later on, but not to have 
> > any side effect. This would just be the way to implement it I 
> > described in my mail.
>
>
> hmm, I don't understand what you mean here, precisely I don't see any 
> interest to have -setType: called in NSButtonCell when you take in 
> account the fact that it is impossible to avoid side effect if you don't 
> override -[NSCell setType:] in NSButtonCell. In the case you choose to 
> override -[NSCell setType:] it with a method which is empty or not, 
> could you explain me the interest to have it called at the NSButtonCell 
> level… ?

The reason to do it this way is object orientation :-)
If we use methods even inside of our own code, we are able to change the 
behaviour in subclasses without overwriting the main methods themselves, 
just by overwriting the helper method. This would give all the 
subclasses of NSCell a chance to implement their own setType: logic and 
they would only have to do so once. This does not come for free, doing 
the attribute changes as method calls costs runtime, which is why we did 
not do it up to now. But knowing that NSButtonCell requires a different 
behaviour seems enough reason to change it. (Nicola, I hope you agree on 
this. I think even when loading a 1000 cells in a browser and changing 
their attributes would not be noticable slower.)

I expect that you know all of this as good as I do. Would you otherwise 
have choosen to work in a fully object orientated language like 
Objective-C? It is this power of overwriting superclass implementations 
which make late time binding fun. And sometimes even calling an empty 
method is a good thing. :-)

If you want some more explaination, just ask again and I try to show it 
with some example code. This is an important point, so if in doubt ask.



BTW, the problems that Alexander was pointing to are based on the fact 
that NSButtonCell is not a direct subclass of NSCell but of 
NSActionCell. This class takes care that all changes of visible 
attributes are reflected in the display. When you did overwrite the 
superclass call with a direct ivar assignement you left out this bit.
He is also right that NSButton should not do most of the 
setNeedsDisplay: calls it does for attribute changes. It is the 
responsibility of the cell to inform the owning control or button. But 
this is already unclear in the NSControl implementation.