On 14 February 2013 14:37, Igor Stasenko <[email protected]> wrote: > On 14 February 2013 07:35, Nicolas Cellier > <[email protected]> wrote: >> 2013/2/13 Igor Stasenko <[email protected]>: >>> On 14 February 2013 00:15, Nicolas Cellier >>> <[email protected]> wrote: >>>> Please use my MessageTally trick above and you'll unfortunately see >>>> that intersect: is just one producer of empty Rectangle among many... >>>> >>>> So I think that the ifNone: protection is quite vain given the flow of >>>> empty rectangles, user is not in control at all, it's an illusion, and >>>> that a reasonnable decision is to enforce the invariant. >>>> >>> >>> but the way to enforce it is to make rect to have consistent answers >>> to #width and #height, >>> regardless in what order of origin:corner coordinates given. >>> First , you make rectangle to be consistent.. then the users of it.. >>> i do not see other way to deal with it.. >>> >>> >>> I repeat, the final intent was to change #setOrigin:corner: to: >>> >>> Rectangle>>setOrigin: topLeft corner: bottomRight >>> origin := (topLeft x min: bottomRight x) @ (topLeft y min: >>> bottomRight y). >>> corner := (topLeft x max: bottomRight x) @ (topLeft y max: >>> bottomRight y). >>> >>> like that, there is no way a #width or #height can answer negative values. >>> >> >> What I just want to remind you is that the base system is not at all >> ready for such a change and that you have a lot of work to clean it of >> degenerated Rectangle, it's not just cleaning intersect: and >> intersects: >> >> Are you sure you aint' gonna need an abstraction to represent an empty >> Rectangle? >> What if I try to inset a rectangle by more than half extent? >> What if I explicitely create a Rectangle with negative extent? >> (0@0 extent: -10@10) extent = (10@10) does not necessarily better meet >> my expectations... >> > negative extent? That should raise an error imo. > >> So what is your plan? >> - don't provide empty rectangle abstraction but equip each and every >> Rectangle producer with an ifEmpty: parameter? >> - create a class EmptyRectangle class with more explicit semantic than >> hasPositiveExtent not? >> > no. I do not see why we need such thing. We could have "isEmpty" or > "isDegenerate" tests, > to let users check this if necessary.. but then consider our code base: > - where you will put these tests? > I will give you a clue: in places where code doing wrong things with rects.. > That means, you are gonna to fix that code anyways. > >> Of course, if you can come with a cleaner model (less obscure >> features, and avoiding spreading guards everywhere in client code) >> then it is better to change the model rather than patch it. >> >> But it's not clear at all to me what your strategy is and where I can >> read about it? >> The bug reports I read are not informative and rather scary because >> they just completely ignore the old invariants. It's not a recipe for >> providing smooth transition. >> >> Also, when you change the invariants in the base system you have to >> provide a transition period to support the migration of client code. >> >> IMO, in the transition period, the code should better raise an Error >> rather than fixing things to what you THINK that is right in total >> contradiction with what users KNEW was right in the previous version. >>
and yes, about raising an Error.. this is exactly what i did by replacing: rect1 intersect: rect2 with: rect1 intersect: rect2 ifNone: [ self error: '..... do something about it' ]. so where we have a disagreement about it? yes.. the thing is not finished.. still the changes we did going towards right direction. just need more work. > wait, what was right? that intersection of two non-intersecting > rectangles gives you some rectangle? > > or that insetting 2-pixels wide rectangle by 5 pixels should produce > some useful shape? > > btw, do you know that after i replaced intersect: with #intersect:ifNone:, > i got a small, yet noticeable world redraw speedup. > >> What are your plans concerning transition? > > The "strategy" is pretty simple ;) : > - change the behavior > - see where it breaks > - fix broken stuff > >> >> Nicolas >> > > -- > Best regards, > Igor Stasenko. -- Best regards, Igor Stasenko.
