Hi nicolas Thanks for the questions. Indeed we should get a strategy. I like your idea of raising an exception. Now how would you do it? I'm so sad that the system is build on sand.
Stef > 2013/2/13 Igor Stasenko <siguc...@gmail.com>: >> On 14 February 2013 00:15, Nicolas Cellier >> <nicolas.cellier.aka.n...@gmail.com> 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... > > 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? > > 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. > > What are your plans concerning transition? > > Nicolas > >> But as i said, for that we needed to review all the code which was assumed >> that >> rectangles internally keep origin @ corner points as they supplied >> during construction.. >> but this is inherently wrong assumption.. one should not put any >> assumptions on object's >> internal storage. never! only on it's desired behavior. >> And desired behavior for rectangle with two points (0,0) and (10,10) >> regardless in what order >> they given to answer 0 for #left, and 10 for #right. >> >> Because once we make assumptions about internal storage then what >> prevents me from assuming that following should also work: >> >> extent := 10@10. >> rect := Rectangle origin: 10@10 extent: extent. >> >> self assert: extent == rect extent >> >> Doing the small patches to fix minor stuff was not an intent of this >> refactoring.. >> because it is counterproductive as to me: if model is wrong and >> assumptions is wrong, >> you will meet problems with it over and over again.. >> and as result, in a longer term you will spend more time fixing >> glitches and dealing with inconsistencies here and there comparing to >> just fixing the model and putting right assumptions in a first place, >> once and for all. >> This is the Pharo spirit :) well, at least to me :) >> > > Agree, if you have enough power to do it. > >>> It's not ideal, but it's a wiser decision in the short term. >>> >>> Nicolas >>> >>> 2013/2/13 Igor Stasenko <siguc...@gmail.com>: >>>> On 14 February 2013 00:01, Nicolas Cellier >>>> <nicolas.cellier.aka.n...@gmail.com> wrote: >>>>> So here is what I did: >>>> ..... >>>> >>>>> >>>>> My feeling is that you are not thru with degenerated rectangles :) >>>>> You'd better enforce the invariant in >>>>> http://code.google.com/p/pharo/issues/detail?id=2601 >>>>> rathert than remove it as requested in >>>>> http://code.google.com/p/pharo/issues/detail?id=2441 >>>> >>>> After replacement of intersect: with intersect:ifNone: >>>> there is no need to enforcing invariants in #intersect: anymore, because >>>> users can decide by themselves what to do if two rects are not >>>> intersecting.. >>>> >>>> the behavior of #intersect: for non-intersecting rects is totally >>>> absurd and changing it to answer empty rect >>>> (like with all zeroes) is not a good solution either, comparing to >>>> providing a closure to handle it. >>>> >>>>> >>>>> Nicolas >>>>> >>>> >>>> >>>> -- >>>> Best regards, >>>> Igor Stasenko. >>>> >>> >> >> >> >> -- >> Best regards, >> Igor Stasenko. >> >
