Re: [boost] Boost Graph library - r_c_shortest_paths

Subject: Re: [boost] Boost Graph library - r_c_shortest_paths
From: Andrew Sutton (andrew.n.sutton_at_[hidden])
Date: 2010-02-22 13:07:06

> I checked the files I sent you and they were the correct versions (all
> three code files: the test file, the example file, and
> r_c_shortest_paths.hpp). The test and example files both run without any
> problems on my system (WIN XP 32 SP 3, MS Visual C++ 2008).
>

I've finally had some time to go through the resubmission. There are/were a
number of issues that need to be addressed.

First, property maps,  functors, and visitors  are virtually always passed
by value. If those objects need to maintain complex state between calls,
it's typically done by building a functor (visitor, etc) that has a
reference to an external state object. I fixed this.

Second, I removed the ks_smart_ptr class, which turned out to be not very
smart. It only wrapped the pointer, but didn't manage it in any way. I also
cleaned up some of the allocation/deallocation code by moving the calls to
two new functions create and destroy.

Inidentally, valgrind shows that this implementation leaks memory. I put
cout's in create and destroy to count allocations and deletions (16646
allocs to 15994 deallocs). My guess is that one of the rarer code paths in
the dispatch doesn't actually delete. This needs to be investigated.

Third... just a style point... some of the identifiers are a little long. I
think the readability could be improved by shortening them.

Fourth, and I'm hoping to get Jeremiah's input on this... I'm not a fan of
passing vector<vector<>>& or vector<>& as top-level parameters. It might be
better to require a template parameter that is essentially a matrix- or
vector-like object. That would simplify the call interface and allow a
little more freedom w.r.t. parameter types. Thoughts?

Fifth... and Final... The interface to the algorithm is changing which can
break backwards compatibility. Maybe we need to give the new module a
different name. Thoughts?

Here's a tarball with the code ready to be dropped into Boost. You should
refer to this version since its what I'm going to be merging.

http://www.cs.kent.edu/~asutton/rcsp.tar.bz2

Andrew Sutton
andrew.n.sutton_at_[hidden]