[Clam-devel] Re: Patch for Steps of LoadFrom

[David García Garzón]

I like this final step. Great! Just to comments:

&ptr[size] is equivalent to the more readable ptr+size

I would prefer if the load test does not relay in StoreOn to do the assert. I 
would prefer you to assert on the boundsAsString method because:
1- it is a lot more cheaper in cpu
2- if the storage fails, current assert will make both the load and the store 
tests will fail instead of just failing the store test


On Dilluns 16 Juny 2008, JunJun wrote:
> Patch of LoadFromStepEnd:
> delete the LoadFrom method in derived classes and implement it in
> Segmentation, and pass unittests.
>
> By now, the maxPosition is just set on the constructor. Here we
> would need to change the XML format to add such a bound as part of the XML.
> Could you schedule a meeting please?
>   Patch of LoadFromStep8:
>   implement LoadFrom() in DiscontinuousSeg and UnsizedSeg,
>   and pass unittests TestLoadForm.
>     Patch of LoadFromStep7:
>     implement LoadFrom() in CongtiguousSegmentation,
>     and pass unittests.
>
>       Patch of LoadFromStep6:
>       add UnsizedSegmentation::addArray(marks, marks+nMarks); and pass
> test. and confirmed that UnsizedSegmentation s(200,marks,marks+nMarks); is
> equivalent to UnsizedSegmentation s(200); s.addArray(marks, marks+nMarks);
>
>
>         Patch of LoadFromStep5:
>         to test whether DiscontinuousSegmentation
> s(200,marks,marks+nMarks); is equivalent to DiscontinuousSegmentation
> s(200); s.addArray(marks, marks+nMarks); Not only in testStoreOn(), but
> also other related tests.
>           Patch of LoadFromStep4:
>           add DiscontinuousSegmentation::addArray(marks, marks+nMarks); and
> pass test Patch of LoadFromStep3:
>             to test whether ContiguousSegmentation
> s(200,marks,marks+nMarks); is equivalent to
>             ContiguousSegmetation s(200); s.addArray(marks, marks+nMarks);
>             Not only in testStoreOn() but also in other related tests.
>               Patch of LoadFromStep2:
>               to test whether ContiguousSegmentation
> s(200,marks,marks+nMarks); is equivalent to ContiguousSegmetation s(200);
> s.addArray(marks, marks+nMarks);
>
>               Bests,
>               June
>                 ----- Original Message -----
>                 From: JunJun
>                 To: David García Garzón
>                 Cc: clam-devel at llistes.projectes.lafarga.org
>                 Sent: Sunday, June 15, 2008 10:39 PM
>                 Subject: Patch for Steps of LoadFrom
>
>                 > Take just ContiguousSgmentation:
>                 >
>                 > We would like to have a virtual function that receives
>                 > the array. But current implementation does not receive a
>                 > CLAM::Array but it is templated as iterators which is
>                 > very convenient for other uses (tests). But template and
>                 > virtual are incompatible, the first refactoring would be
>                 > to turn the template constructor into a constructor
>                 > receiving just TData* which i think is compatible with
>                 > any current use. Just in case, compile, both the
>                 > annotator and the tests and pass the tests.
>                 >
>                 > Once we turned the constructor into a templateless one,
>                 > lets do an 'Extract method' refactoring from the
>                 > constructor for the data filling part. So, for each
>                 > Segmentation add the following method:
>                 > void takeArray(TData * begin, TData * iterator end)
>                 > {
>                 >  // copy here the filling code from the constructor
>                 > }
>                 > Then substitute the copied code at the constructor by a
>                 > call to such method. All the test should be working.
>
>                 The patch is of the step above.
>                 Notice that to make the test pass, I made a forced type
> change (Iterator->TData), and changed the related type (double->float) of
> the unittests. That is ugly and requires the "Iterator" datatype must be
> consistent with "TData".
>
>                 > No proceed the same with the rest of the Segmentations
>                 > sending patches at the same points.
>                 >
>                 > Create a virtual method takeArray on base Segmentation.
>                 > Should still compile.
>                 >
>                 > On SegmentationPane, intead of using the three parameters
>                 > constructor use the one parameter one and let's call the
>                 > takeArray method after the switch. All the segmentations
>                 > should still work so try them at Annotator as we did some
>                 > days ago.
>                 >
>                 > Now, those refactoring have prepared the way for our
>                 > desired red, the LoadFrom test. Some hints: you can
>                 > define an std::istreamstream("With some xml content to be
>                 > read") and see how previous tests use the boundsAsString
>                 > method to get the actual segmentation content as string.
>                 >
>                 > Now you'll try to use the load but now we have a problem:
>                 > We are missing the audioDuration parameter. By now, just
>                 > set it on the constructor. Here we would need to change
>                 > the XML format to add such a bound as part of the XML,
>                 > this can break a lot of things so this is the perfect
>                 > point to have a meeting.
>                 >
>                 >
>                 > --
>                 > David García Garzón
>                 > (Work) dgarcia at iua dot upf anotherdot es
>                 > http://www.iua.upf.edu/~dgarcia



-- 
David García Garzón
(Work) dgarcia at iua dot upf anotherdot es
http://www.iua.upf.edu/~dgarcia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20080616/f42ef069/attachment-0001.sig>