[boost] boost::geometry #3

From: Aleksey Gurtovoy (alexy_at_[hidden])
Date: 2000-01-26 08:28:58

Sorry for a long delay. I had some hard times (and actually still have them
now). But anyway - I've updated geometry classes with the modifications we
discussed some time ago, and have uploaded the new version to the code
vault.

There are two major changes in the code:
(1) second template argument was added to all classes
(2) operator < was replaced with specialization of std::less

There are also some minor fixes and modifications, but they are probably too
small to discuss now - you may want to look at the code first.

The good (or bad, I don't know ;) thing about the delay is that I've had a
chance to use these classes in a real code. Below are some thoughts I've
came away with as the result of this experience.

Summary: probably I agree with the first Dave's reply about an redundancy of
'delta' (or 'size') class and about a clutter it may create in the code.
Problems I've faced with are not very big (probably someone would not think
they are problems
at all), but I certainly don't like things as they are now. So I would like
to hear your opinions about the issue.

A small example
----------------
Suppose we have a canvas class, which supports a viewport concept, e.g.

| class canvas {
|  public:
|    typedef boost::geometry::delta<long> delta;
|    typedef boost::geometry::point<long> point;
|    // other members
|
|    const point& viewport_origin() const;
|    void viewport_origin( const point& p );
|  };

The very first try of use this interface will lead to a problem:

| canvas c;
| canvas::point new_origin;
| // ...
| c.viewport_origin( c.viewport_origin() + new_origin ); // doesn't
compiles!

This is because we can't add two points - only a point and a delta or two
deltas (see geometry.hpp header for the table of legal and illegal
operations). These rules seem to be reasonable, but sometimes the result is
confusing, and this is certainly such a case...

So you've decided to reflect some time on the problem and soon came with the
following fix:

| class canvas {
|  public:
|    // ...
|    const delta& viewport() const;
|    void viewport( const delta& p );
|  };

now we can write

| c.viewport_origin( c.viewport_origin() + new_origin );

but... other problems will arise sooner or latter.
For instance, in the same project you have an 'animal' class, which can
paint itself on canvas and you want to simplify these paint operations for
derived  classes, allow them to deal only with local coordinate system...

If you are like me, you will try to write something like this:

| void
| animal::paint( canvas& c )
|  {
|  canvas::delta old_origin = c.viewport_origin();
|  c.viewport_origin( location().top_left() ); // (*) doesn't compiles!!!
|  do_paint( c );
|  c.viewport_origin( old_origin );
|  }

Now we have to re-write line (*) as

| c.viewport_origin( location().top_left() - canvas::point() );

Probably this is not too bad, but at least it's tiresomely...

And there are some other non-consistency which makes me to feel myself
quite dissatisfied.
E.g. we can't add two points directly, but we can do this with either
coordinates taken separately:

| canvas::point p1, p2;
| p1 = p1 + p2; // illegal
| p1.x() = p1.x() + p2.x(); // ok
| p1.y() = p1.y() + p2.y(); // ok
| p1 = canvas::point( p1.x() + p2.x(), p1.y() + p2.y() ); // ok

Or we can't compare a point and a delta directly too, but again we can
compare their coordinates taken separately...
and so on..

We can fix the situation in two ways - by re-designing current interface in
order to resolve the problems (I'm not sure it's possible to resolve all of
them, but I see how we can manage with a few of them), or just by removing
'delta' class.

Your thoughts?

- Alexy