On 14 February 2013 00:15, Nicolas Cellier <nicolas.cellier.aka.nice@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. 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 :)
It's not ideal, but it's a wiser decision in the short term.
Nicolas
2013/2/13 Igor Stasenko <siguctua@gmail.com>:
On 14 February 2013 00:01, Nicolas Cellier <nicolas.cellier.aka.nice@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.