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