quadtree icon indicating copy to clipboard operation
quadtree copied to clipboard

Simplify `Area` API and get rid of `derive_builder`

Open JohnDowson opened this issue 1 year ago • 2 comments

JohnDowson avatar Dec 01 '23 13:12 JohnDowson

I don't like that Area::new(width: U, height: U) constructs an area anchored at (1,1) by default. That feels like a footgun to me. One of the things I liked about the builder pattern and derive_builder is (was?) that it makes it hard to forget to do things like call .at_point().

Is this API change a necessary prerequisite to the overlapping/nonoverlapping changes you want to make down the line? If not, I'd prefer to leave this as it is. If so, maybe you can help me understand what needs to change first before we can make the overlapping/nonoverlapping changes you really want to make.

ambuc avatar Dec 01 '23 16:12 ambuc

While I see your argument, that's directly opposite of the pattern's usual application: making it easier to build types that have many optional fields.

No, it is not a prerequisite, just what I deem to be an improvement in ergonomics.

Alternative solution to reminding people to construct correct areas I see, is making the only way to constrict an Area:

let point = ...;
let area = point.expand_to(w, h);

JohnDowson avatar Dec 01 '23 19:12 JohnDowson