Hi Subbu.

> I saw direct ivar access instead of the fontsToUse from other places also.

Looks fine. The few ivar reads have ifNil-checks. ivar write is only in #initialize* and #set* methods. Calling #font will never return "nil". So "self font textStyle" is fine.

> How about this patch?

Ah, well. You are right. The internals of StringMorph can deal with both font and emphasis being nil. The goal would be to reduce the number of #ifNil checks. If somebody would send #setFont:emphasis: with nil and nil -- which would not be okay because (1) #set* is rather internal initialization code and (2) one of both should be non-nil -- everything would work. However, if somebody would send #font:emphasis: with nil and nil, then the error message is awkward.

You assertion looks good. I will add comments, too.

Best,
Marcel
Am 25.05.2020 10:18:40 schrieb K K Subbu <kksubbu.ml at gmail.com>:
On 25/05/20 12:49 pm, Marcel Taeumel wrote:
> Well, either aFont or emphasisCode can be nil. Not both. ;-) If I recall
> correctly, that's the existing usage I found when refactoring
> StringMorph last year in summer. It would be strange to support both
> arguments to be nil.

StringMorph>>initializeFromText uses 'self font textStyle' without
checking for nil. I saw direct ivar access instead of the fontsToUse
from other places also.

How about this patch?

self assert: (aFont notNil | emphasis notNil).
font := aFont.
emphasis := emphasisCode ifNil: [0].

Regards .. Subbu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200525/5a761bbe/attachment.html>