[Clam-devel] [PATCH] segmentation in chord extractor as a seperate class

[David García Garzón]

On Wednesday 25 July 2007 00:39:17 Roman Goj wrote:
> Accessors are here :)
>
> There's only one problem - the closing of the last segment - I'd like
> to make _lastChord local for ChordExtractorSegmentation, not passed
> with doIt (or is this not the right way?). So I'd like to be able to
> check whether there is need for the closing of the last segment from
> inside ChordExtractorSegmentation, so I added this:
>
>         void closeLastSegment(unsigned & lastChord, CLAM::TData &
> currentTime ) {
>                 if (lastChord != 0 & _timePositions.size() != 0)
>                 {
>                         CLAM_ASSERT(_timePositions.size()%2==1,
> "Attempting to close last segment even though all segments have been
> closed");
>                         _timePositions.push_back(currentTime);
>                 }
>         }
>
> to be called from the dumper's destructor... any nicer way of doing this?

Well, this call has no sense at all when we move to a streaming context. So in 
any case we should deal with this case when visualizing segments. Having a 
closing segment point is a pool requirement. But i am not sure that this 
should be a requirement for the streaming representation. Keep it like that 
and let's see later as we move on.


> Also - removed the commented out debugging code I introduced in the
> first patch... further cleaning up coming... :)
>
> Changelog:
> * added accessors to _timePositions and _chordIndexes in
> ChordExtractorSegmentation
> * added closeLastSegment in ChordExtractorSegmentation
> * removed commented out debugging code in ChordExtractorSegmentation

Nice, committing.

> One more doxygen question though...
> If I make comments like this
>
> // This line does something extra neat
> Cool.extraNeat( new wicked );
>
> Should I make them also doxygen comments (or is doxygen supposed to be
> used, as I'm guessing, just for variable/function declarations?) I can
> check I guess somewhere... ok, found it:

No. Inner comments are not parsed by doxygen. You should think that doxygen 
comments are the ones that appear here:
http://clam.iua.upf.edu/doc/CLAM-doxygen/

Besides that, in CLAM, we consider that commenting inner code a bad smell of 
having obscure code. So we have the rule that instead commenting inner code, 
we should make it clearer, by using proper variable names and methods. One of 
the smarter rules (that you can find in Martin Fowler's Refactoring book) are 
when you comment some code telling that 'it does someting', wrap it into a 
method called DoSomething and get ride of the comment.

While commenting inner code is considered a bad smell, commenting method with 
doxygen is a very good thing as they allow a developer to use this method as 
a blackbox.

> "Doxygen allows you to put your documentation blocks practically
> anywhere (the exception is inside the body of a function or inside a
> normal C style comment block)."

So you answered your own questions. :-) Anyway this has given me the 
opportunity of advicing you about inner code comments.

> in the manual link on doxygen.org, look up time 5 min. No RTFMs for me
> this time then I guess ;-)