On Tue, Apr 18, 2017 at 10:39:34PM +0530, K K Subbu wrote: > On Tuesday 18 April 2017 09:51 PM, Alistair Grant wrote: > >This appears to be changing the meaning of #ensureEndsWith:. > >The previous meaning was that it always ends with the supplied object. > > The method name is ambiguous. If the collection is empty does it need an > "ending"? All its senders use it for line/statement ending. If cursor is at > the beginning of a collection, this becomes a null-op. This interpretation > is also consistent with that of Squeak.
Being consistent with Squeak sounds like a good argument. > >You've modified it to mean that an empty collection doesn't have the > >object appended. > > Yes, based on its usage. Otherwise, every new logger starts with a blank > line :-(. Sure, but... :-) 1. This is assuming that ensureEndsWith: is only used for strings, but the implementation doesn't assume that (I tested it with an Array and it was fine). 2. If you look at existing senders of ensureEndsWith: they check that the string isn't empty. Having said that, I'm not opposed to the change (especially if it improves interoperability between the various Smalltalks), just that we think about the wider implications. Cheers, Alistair > >Is that really what the method is supposed to mean? The comment matched > >the previous implementation, with your change, it doesn't. > > > >If the change does go ahead, the comment should be updated to reflect > >the changed definition. > > Good catch! I will update comments. > > >I'm also wondering why you've changed the test to hard-code a string > >stream? > > I just took a common usecase from CCodeGenerator. > > Regards .. Subbu
