The Trunk: Collections-dtl.548.mcz

On 13 December 2013 12:57, David T. Lewis <[hidden email]> wrote:
> On Fri, Dec 13, 2013 at 07:42:28AM +0000, Frank Shearar wrote:
>> On 12 December 2013 23:58,  <[hidden email]> wrote:
>> > David T. Lewis uploaded a new version of Collections to project The Trunk:
>> > http://source.squeak.org/trunk/Collections-dtl.548.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Collections-dtl.548
>> > Author: dtl
>> > Time: 12 December 2013, 6:58:08.878 pm
>> > UUID: b167353b-b9ab-4911-8168-c5f184cab62a
>> > Ancestors: Collections-ul.547
>> >
>> > Transcripter>>initInFrame: should do nothing when not in MVC
>> >
>> > =============== Diff against Collections-ul.547 ===============
>> >
>> > Item was changed:
>> >   ----- Method: Transcripter>>initInFrame: (in category 'initialization') -----
>> >   initInFrame: rect
>> > +       Smalltalk at: #Paragraph ifPresent: [:classParagraph | "MVC"
>> > +               frame := rect insetBy: 2.  "Leave room for border"
>> > +               para := classParagraph withText: self contents asText
>> > +                                       style: TextStyle default
>> > +                                       compositionRectangle: ((frame insetBy: 4) withHeight: 9999)
>> > +                                       clippingRectangle: frame
>> > +                                       foreColor: self black backColor: self white]!
>> > -       frame := rect insetBy: 2.  "Leave room for border"
>> > -       (Smalltalk hasClassNamed: #Paragraph)
>> > -               ifFalse: [^ self inform: 'MVC class Paragraph not present'].
>> > -       para := (Smalltalk at: #Paragraph) withText: self contents asText
>> > -                               style: TextStyle default
>> > -                               compositionRectangle: ((frame insetBy: 4) withHeight: 9999)
>> > -                               clippingRectangle: frame
>> > -                               foreColor: self black backColor: self white!

>>
>> Well, that definitely stops a DNU and such when running in Morphic
>> (yay!) but it still won't work in a non-MVC/Morphic world. That's when
>> you accused me of overengineering, David :)
>
> Yes you're right, I meant that with a smiley, sorry :)

I knew there was something of a smiley in there :) I just meant that
if we want to preserve the Transcripter's emergencyEvaluator we would
need this "complexity" because we'd need an emergencyEvaluator per UI
framework.

> Dave, the only thing I don't like about this change is I thought we said we
> were going to migrate away from using the Dictionary API on Smalltalk.  This
> was back when Andreas and community came up with the idea of the wrapped
> 'globals'.
>
> I think it would be better to use
>
>     (Smalltalk classNamed: #Paragraph) ifNotNil: [ : classParagraph | ... ]
>
> otherwise we can never find all the places where we try to do logical access
> to class.

In which case we should have #bindingOf:ifPresent, or similar. An API
that forces you to acknowledge the potential not-there-ness of a thing
is far preferable, in my book, to a separate-check-then-use. The
latter also have fun time-of-check-time-of-use issues in a
multiprocessing environment... which we happen to have.

>> >> Well, that definitely stops a DNU and such when running in Morphic
>> >> (yay!) but it still won't work in a non-MVC/Morphic world. That's when
>> >> you accused me of overengineering, David :)
>> >
>> > Yes you're right, I meant that with a smiley, sorry :)
>>
>> I knew there was something of a smiley in there :) I just meant that
>> if we want to preserve the Transcripter's emergencyEvaluator we would
>> need this "complexity" because we'd need an emergencyEvaluator per UI
>> framework.
>
>
> -1.  Please be more conservative and lazy about putting new code/elements
> into the system.  TSTTCPW demands that we don't start building out
> infrastructure for new UI frameworks until those frameworks are being pushed
> into existence and such a factoring is actually NEEDED.  Please only write
> code for the "now" not for some "potential future".  10 years could go by
> with no new graphical frameworks and then someone looking at your
> class-hierarchy will regard it as "cruft".

(a) The emergency evaluator's there _for good reason_.
(b) We HAVE two UI frameworks right now. They're part of the standard image.
(c) "Stripped" 4.5 base images do not have ST80, so this code is broken.
(d) To fix it _requires_ two separate UI implementations because of point (b).

Unless you're specifically saying "we should not have an emergency
evaluator". But then say that, not -1 me suggesting a way to fix
something broken. (Please feel free to suggest a simpler way of
solving the problem that works for all CURRENT UI frameworks.)

> Because there has been no concrete vision/requirement for a Morphic-based
> emergency evaluator (EE).  EE is a relatively rare occurrence (one would
> hope!) and for developers only.  But developers typically use _development
> images_ which tend to be fat, not "Stripped", and so might probably include
> ST80 anyway..?  The cases seem pretty uncertain and so that's why I'm
> questioning engineering new stuff like this into the image.

This is a bug. It needs to be fixed. It's not new functionality,
unless by "new" you mean "it actually works in Morphic".

> Particularly we're at a time when I want to get 4.5 out the door, so that
> may be contributing to my aversion to new engineerings.  I ask you to at
> least consider simple stub error-handling in situations where
> packages/functionality would be unfulfilled.  We can fill in implementations
> later, if needed, but at least you'll have made the structure.

So you'll be happy with

* move the current EE to ST-80,
* fix the EE for Morphic (at some point),
* add an AppRegistry for the EE (which is just a hook like ToolSet and
friends, so we don't tangle the two UI frameworks we have so carefully
untangled)

? If there's a simpler solution, I'm all ears.

On Fri, Dec 13, 2013 at 06:37:19PM +0000, Frank Shearar wrote:
> On 13 December 2013 16:59, Chris Muller <[hidden email]> wrote:
> >> > Dave, the only thing I don't like about this change is I thought we said
> >> > we
> >> > were going to migrate away from using the Dictionary API on Smalltalk.
> >> > This
> >> > was back when Andreas and community came up with the idea of the wrapped
> >> > 'globals'.
> >> >
> >> > I think it would be better to use
> >> >
> >> >     (Smalltalk classNamed: #Paragraph) ifNotNil: [ : classParagraph |
> >> > ... ]
> >> >
> >> > otherwise we can never find all the places where we try to do logical
> >> > access
> >> > to class.
> >>
> >> In which case we should have #bindingOf:ifPresent, or similar. An API
> >> that forces you to acknowledge the potential not-there-ness of a thing
> >> is far preferable, in my book, to a separate-check-then-use. The
> >> latter also have fun time-of-check-time-of-use issues in a
> >> multiprocessing environment... which we happen to have.
> >
> >
> > I used to prefer Potential-not-thereness API's in "my book" too until one
> > day I found it unnecessarily expanding and complicating API's.  Users of
> > API's must write defensive code anyway.  The ifNotNil: approach allows a
> > simpler API that can provide either existence-testing OR access via one
> > simple message rather than two.
> >
> > As for "fun time-of-check-time-of-use issues" that's just bunk.  You think
> > the ifPresent: method doesn't do the same ifNotNil: check or could not be
> > interrupted?  Unless you're thinking to put a Mutex into SmalltalkImage to
> > make it "thread safe".  What, for background unloading?  If you need that
> > guard it from the outside.
> >
> >>
> >>
> >> >> >> Well, that definitely stops a DNU and such when running in Morphic
> >> >> >> (yay!) but it still won't work in a non-MVC/Morphic world. That's
> >> >> >> when
> >> >> >> you accused me of overengineering, David :)
> >> >> >
> >> >> > Yes you're right, I meant that with a smiley, sorry :)
> >> >>
> >> >> I knew there was something of a smiley in there :) I just meant that
> >> >> if we want to preserve the Transcripter's emergencyEvaluator we would
> >> >> need this "complexity" because we'd need an emergencyEvaluator per UI
> >> >> framework.
> >> >
> >> >
> >> > -1.  Please be more conservative and lazy about putting new
> >> > code/elements
> >> > into the system.  TSTTCPW demands that we don't start building out
> >> > infrastructure for new UI frameworks until those frameworks are being
> >> > pushed
> >> > into existence and such a factoring is actually NEEDED.  Please only
> >> > write
> >> > code for the "now" not for some "potential future".  10 years could go
> >> > by
> >> > with no new graphical frameworks and then someone looking at your
> >> > class-hierarchy will regard it as "cruft".
> >>
> >> (a) The emergency evaluator's there _for good reason_.
> >> (b) We HAVE two UI frameworks right now. They're part of the standard
> >> image.
> >> (c) "Stripped" 4.5 base images do not have ST80, so this code is broken.
> >> (d) To fix it _requires_ two separate UI implementations because of point
> >> (b).
> >>
> >> Unless you're specifically saying "we should not have an emergency
> >> evaluator". But then say that, not -1 me suggesting a way to fix
> >> something broken. (Please feel free to suggest a simpler way of
> >> solving the problem that works for all CURRENT UI frameworks.)
> >
> >
> > Ok, all I'm saying is, part of your work is about reorganizing, and other
> > parts are about engineering new functionality just to accomodate the
> > reorganizations.  (What's worse is when you want to remove functionality
> > just to accomodate but I'll be lazy about addressing that).
> >
> > Whenever there's an arbitrary piece of new functionality (e.g.,
> > Morphic-based emergency evaluator) just to avoid crossing package boundaries
> > you don't want to cross, then engineering "solutions" to that I think could
> > end up being code for ghost situations.
> >
> > Because there has been no concrete vision/requirement for a Morphic-based
> > emergency evaluator (EE).  EE is a relatively rare occurrence (one would
> > hope!) and for developers only.  But developers typically use _development
> > images_ which tend to be fat, not "Stripped", and so might probably include
> > ST80 anyway..?  The cases seem pretty uncertain and so that's why I'm
> > questioning engineering new stuff like this into the image.
>
> This is a bug. It needs to be fixed. It's not new functionality,
> unless by "new" you mean "it actually works in Morphic".
>
> > Particularly we're at a time when I want to get 4.5 out the door, so that
> > may be contributing to my aversion to new engineerings.  I ask you to at
> > least consider simple stub error-handling in situations where
> > packages/functionality would be unfulfilled.  We can fill in implementations
> > later, if needed, but at least you'll have made the structure.
>
> So you'll be happy with
>
> * move the current EE to ST-80,
> * fix the EE for Morphic (at some point),
> * add an AppRegistry for the EE (which is just a hook like ToolSet and
> friends, so we don't tangle the two UI frameworks we have so carefully
> untangled)
>

No.

> ? If there's a simpler solution, I'm all ears.
>

Hopefully there is a simpler solution, aalthough I cannot look into it right now.

Dave

> frank