[boost] Re: geometry classes code [not so long ;)]

From: Dave Abrahams (abrahams_at_[hidden])
Date: 2000-01-02 17:08:15

>> Okay, I'm putting my foot down: we are going to have to implement all of the
>> 2-type versions of the operator templates with this workaround for VC++
>> compatibility anyway. We can simply derive the specializations from the
>> versions with the '2' for those platforms which support that syntax.
>> Question: should we deprecate the use of the specialization (LEGAL C++
>> LANGUAGE FEATURE) so that we can one day simplify the library!??!

> I don't think we must take the line of least resistance - especially against
> compiler writers ;)

Yes, but I need the '2' versions of these facilities, too, because sadly I
have to use VC++. So we need to just bite the bullet and supply all of them.
Now if we want our software to run on VC++ and be portable, we need to use
the '2' versions. So who will bother with the other versions if they want
portaability?

<sound of gnashing teeth>

> Well, I agree with you... but which the x/y interface did you mean - one which
> returns non-const references to 'unit_type' or another, that returns non-const
> references to 'self'? ;) (I think you meant unit_type& x(); unit_type& y();,
> but I will be better to ask).

I meant the one returning unit_type&.

> What do you think about a chaining of member function calls in general? I have
> never heard anything about this subject from any person who can made me change
> my mind, so I'll glad to hear something about it from members of this group.

It's unfamiliar to me at least. There is a reason that it is often
recommended that operator+= should return a const reference instead of a
full reference: for compatibility with the built-in types. IOW, you can't
write: int x = 0; (x += 3)++

>> It certainly looks at first glance like there's an opportunity to save some
>> code duplication by factoring parts of point and size into a common base
>> class.

> I thought about it, but by the time I was posting the code, it seems
> non-obvious that we would decide to deal with 'size' at all, so I left the
> refactoring until the best times.

Makes sense. And replying to other your email:

>> These nice parallels notwithstanding, I worry about using the name 'size'
>> this way since it is used for the STL containers to mean something different.
>> It really does mean something more like "difference" in this case, so I guess
>> my preference for 'extent' doesn't work too well either.

>> Suppose we called it 'delta' (ick)?

>> I suggest the member function name 'extent' should be used to get the size of
>> a rectangle, and should return a delta.

> Actually I think that 'extent' as an alternative to 'size' is pretty good. But
> 'delta' is not bad too, so I'm not sure which of them we must choose... And
> member function 'size' will be definitely renamed to 'extent'.

I'm not settled on an answer yet, either.

>>> rect( const point<unit_type>& origin, const size<unit_type>& rect_size );
>>> rect( const point<unit_type>& origin );

>> What does the above constructor do? It constructs an empty rectangle with the
>> top-left point == 'origin' ;) I found this useful some time ago, but I'm
>> beginning to think on your part - this is redudant.

I guess my point was that it was non-obvious from reading the header what it
was supposed to do.

>>> const point<unit_type>& left_top()      const; const point<unit_type>&
>>> right_bottom()  const;

>> I don't know why, but I always say top_left and bottom_right. I think this
>> order is more natural for native English speakers. Do these names seem weird
>> to anyone else?

> I'm not a native English speaker (as probably you've noticed already), so I
> can't say anything about this subject ;). But what other people think about
> this?

>> What about bottom_left and top_right? My rectangles have those, too.

> and mine - they are below in the code ;)

Because you returned non-const references, though, they weren't symmetrical
and lived in a separate part of the file. Anyway, I've convinced you to
change that, it seems.

>> N.B. point and rect are generally cheap to assign, so I would consider
>> eliminating functions like set_empty() which can easily be expressed as 'r =
>> rectangle<T>()'. I tend to favor a more minimal interface, though I can see
>> the readability advantage of set_empty().

> 'set_empty' will be removed.

I might regret that one... ;)

>>> self& origin( const point<unit_type>& new_origin ); self& corner( const
>>> point<unit_type>& new_corner );

>>> self& left_top( const point<unit_type>& new_left_top ); self& right_bottom(
>>> const point<unit_type>& new_right_bottom );

>> I don't think I believe in returning a reference to self from a mutating
>> function. Please convince me.

> Is this an answer to my question about the chaining of member function calls?

Sort of.

>>> // inflating/deflating self& inflate( unit_type delta = 1 ); self& inflate(
>>> unit_type x_delta, unit_type y_delta ); self& inflate( const
>>> size<unit_type>& delta );

>>> self& inflate_size( unit_type delta = 1 ); self& inflate_size( unit_type
>>> x_delta, unit_type y_delta ); self& inflate_size( const size<unit_type>&
>>> delta );

>> What's the difference between inflate_size and inflate? I had to look at the
>> code to figure it out. inflate_size is nondescriptive IMO and unneccessary:
>> r.bottom_right(r.bottom_right + delta) is much clearer.

> I'll remove 'inflate_size'.

I've seen the name 'inset' used for deflate. I don't mind deflate, though.

> And that's all. I'm going to fix all these stuff today and to post a new
> version. Thanks for your time, Dave!!! -Alexy.

Thanks for being open to my suggestions!