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

[David García Garzón]

First of all, the training we did last weeks was about you learning to go 
through a set of little changes in a controlled way instead of changing it 
all, compile and see what it happens. You got in trouble, you deserve 
that ;-) Remember, you are modifying existing, working, very sensible, and 
hard to verify code. Be careful. You could have never reached the stable 
state again, but rolling back changes.

Another issue is that the processing should be independent of the pool. Try to 
construct a Segmentation object (already in clam, do not mix with the 
Segment). Segmentation has several derivatives, choose the proper one.
Feel free also to propose changes to the class interface.

http://clam.iua.upf.edu/doc/CLAM-doxygen/classCLAM_1_1Segmentation.html

After that, the ChordExtractorDescriptionDumper, should take such segmentation 
as processing output and do the actual dump on the pool to serialize.

Each class has its own responsability.



On Thursday 19 July 2007 11:42:34 Roman Goj wrote:
> Hi!
>
> I've started working on chord segmentation - starting offline in
> ChordExtractor. The first step, suggested by David: separating the
> code responsible for segmentation, so that it is all in a separate
> class.
>
> In short:
> Changelog:
> * Most of the segmentation code in ChordExtractor is now in a seperate
> class called ChordExtractorSegmentation
>
> This patch is a "blind man's approach" to the problem - the simplest
> first patch I could think of... I just took the biggest chunk of code
> responsible for segmentation, put it into the doIt of a new class and
> added the constructor. Then I added some other code to doIt, so that
> every variable necessary for the copy/pasted chunk to work has the
> same value as it had in the previous code (basically copy/pasting some
> other lines from the code, without changing them)... then I struglled
> with the "which variable needs to be a reference, which a
> pointer"-mess... ;) And then finally I started getting identical
> results from the old ChordExtractor and the new "improved and much
> more messy" one.
>
> TODO:
> * Clean up! There is some repetitive code now, probably completely
> unnecessary (also - the old code isn't deleted, it's just commented
> out for now). I decided to post the patch without the cleanup, so
> that, if I'm doing something really fundamentaly wrong, someone can
> notice now :)
> * Quite a few variables are now passed to the constructor, probably
> completely unnecessary (like _lastChord), this needs to change
> * there is also some code in the destructor of
> ChordExtractorDescriptionDumper, this needs to go into the new class
> * lots more :)
>
> cheers,
> roman