Re: Implementation of symrcm

[John W. Eaton]

On  7-May-2007, David Bateman wrote:

| Michael Weitzel wrote:
| 
| > OCTAVE_LOCAL_BUFFER doesn't seem to work with type
| > bool - why?! I allocated a vector of bools manually.

It fails because OCTAVE_LOCAL_BUFFER is currently just a macro that
uses std::vector<T> and the C++ standard allows std::vector<bool> to
be defined using a specialized bit vector.

| There are a few issues with this code.
| 
| [...]

| John, if Michael is happy with the changed code, I'd suggest adding this
| to octave core as it is a core function of matlab.. Note that I've
| unconditionally included config.h so that the compile can get done
| outside of the source tree. This should be reincluded when the code is
| committed..

I would also add:

  In C++ there is no need to always write "struct CMK_node".  Instead,
  you can just use "CMK_Node" as the type once the struct is declared.

  Instead of passing pointers to scalar values, please use
  references, and instead of passing a struct by value, you probably
  want to pass it as a const reference if possible.  So for example,
  the function Q_enq should be

    static inline void 
    Q_enq (CMK_Node *Q, octave_idx_type N, octave_idx_type& qh,
           octave_idx_type& qt, const CMK_Node& o)
    {   
      Q[qt] = o;
      qt = (qt+1) % N;
    }

  and the call would be

    Q_enq (Q, N, qh, qt, v);
    

If you are happy with it, please add this function and commit.

Thanks,

jwe