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. > 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.
