On 14 February 2013 07:35, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
2013/2/13 Igor Stasenko <siguctua@gmail.com>:
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.
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.