[geos-devel] re:GEOS

[Mark Coletti]

On Fri, 11 Mar 2005 19:48:06 +0000, Artem Pavlenko
<artem at pavlenko.uklinux.net> wrote:
> I've noticed that there are some changes in CVS and I'm glad to see the
> following:
> 
> LineIntersector::LineIntersector()
> {
>     precisionModel=NULL;
>     Coordinate *c=new Coordinate();
>     intPt[0].setCoordinate(*c);
>     delete c;
>     c=new Coordinate();
>     intPt[1].setCoordinate(*c);
>     delete c;
>     // alias the intersection points for ease of reference
>     pa.setCoordinate(intPt[0]);
>     pb.setCoordinate(intPt[1]);
>     result=0;
> }
> 
> becoming :
> 
> LineIntersector::LineIntersector(): pa(intPt[0]), pb(intPt[1])
> {
>     precisionModel=NULL;
>     // alias the intersection points for ease of reference
>     //pa=intPt[0];
>     //pb=intPt[1];
>     result=0;
> }
 
> But to me, this is still a wrong use of C++. From what I can see, this
> is one example of many, maybe as a result of translating Java into C++?

I'm not that familiar with Java, so I'm curious to know of the
Java-isms you see.

And, yes, the changes that occurred in CVS are an improvement.  Let me
guess what you suggest should happen:


LineIntersector::LineIntersector()
 : precisionModel(0),
   result(0),
   pa(intPt[0]), pb(intPt[1]),
{
	// alias the intersection points for ease of reference
	//pa=intPt[0];
	//pb=intPt[1];
}

That is, as Meyers suggests in _Effective C++_, "Prefer initialization
to assignment in constructors."  He also counsels against using C's
NEW macro. (For that matter, he strongly urges use of consts over
macros in C++.)

I'm concerned about the members pa and pb since they appear to be for
syntactic sugar.  Are they really necessary?  And what's up with the
data members in the protected section?  Doesn't that break
encapsulation?

Also, it's generally good practice to have one class per header
instead of consolidating them into the same lexical space.  Doing so
will likely trigger unnecessary extra recompiling.  "geosAlgorithm.h"
has 26 class declarations -- changing the interface for one will cause
recompilation for ALL external classes dependent on ANY of the classes
found in that header file.

> I also have some questions about future directions. As I see it, there
> can be lots to gain from introducing templates, because:

> 1. you're using them already as part of STL

Yes!

> 2. they might lead to better performance

Yes!

> 3. you'll able to abstract things like memory allocation and precision
> models, allowing clients to configure the library the way they like and
> concentrate on actual algorithms

Yes!

> 4. templates are part C++ ISO!

Yes!  Have been since, what, _1999_?

> And also there is boost.org and you will be dealing with smart pointers
> at some point, so why not now?

Yesyesyesyes!

Boost also has a LOT of other goodies besides smart pointers, though
those are the most useful classes.  I've noticed that a lot of linux
distros now include Boost. (E.g., been with SuSE 9+.)

Check out the list at http://www.boost.org/libs/libraries.htm and drool.

Note, too, that some of the Boost developers are (were?) on the ISO
C++ committee.

Cheers!

Mark
-- 
I'm taking reality in small doses to build immunity.