[Clam-devel] Copy & Paste processings on canvas patch

[David García Garzón]

I think we are blocking Natanael's good pace. The 'hold on' was just because 
the format, as it has deeper implications. But if we agree on the format, 
let's commit whatever solution that gives that format and then refactor the 
implementation until we are happy with it. Do we agree with the XML format? 
Which is the final one?

David.


On Divendres 13 Juny 2008, David García Garzón wrote:
> On Divendres 13 Juny 2008, Pau Arumí Albó wrote:
> > El dj 12 de 06 de 2008 a les 20:02 -0300, en/na Natanael Olaiz va
> >
> > escriure:
> > > Well, sorry for the late patch. I was quite busy with other things, and
> > > in the middle thinking and testing different types and ways to pass the
> > > data between classes.
> >
> > Sure, no need for hurrying.
> >
> > > Here is my actual solution (which I don't know if it's so good): I made
> > > a subclass of network: ProcessingGeometry[1]. I think maybe it would be
> > > simpler with just a structure, or putting it on other place
> > > (Processing?). Any critic is welcome.
> >
> > Ok, I see that you made an "inner" class.
> >
> > * A formatting issue: we follow the rule of adhering to "local"
> > formatting convention. So in Network, let's use Method() instead of
> > method()
> >
> > * I much prefer a simple structure with no getters. But specially
> > without string conversion methods. The conversion to XML should be done
> > in the ProcessingDefinitionAdapter, using XMLAdapter<int>. Much simpler!
>
> If you want to get "4,3" it is better to have the formatting as Natanael
> had and XML adapt strings.
>
> > > On MainWindow I added a line to load the geometries from the XML, and
> > > if it fails try with the .pos file.
> > > I would need to add a checkbox when saving, to select what format do
> > > you want. By now, it write only the traditional .pos file (for
> > > development testings you can paste the clipboard after copy / paste to
> > > a file, and load it).
> >
> > No, I think this features/complexity doesn't pay its cost in this case.
> > Let's do the jump without fear doing:
> >
> > * we'll ask for extensive testing when committed (and revert if
> > something fails).
> >
> > * You could leave the code depending on a constant like this
> > const saveUsingOldPosFiles=false;
> > So that if we find problems, reverting the behavior costs nothing.
> > And when tested all the .pos stuff will be kicked out.
>
> For transition, I would suggest loading first the pos file if it exists as
> the old code does, and then always ask for the geometries. If geometries
> are empty, no problem. If there are any overlap, geometries will rule. On
> saving, always use geometries so, clear the .pos code.
>
> > A different issue:
> >
> > Have you considered not passing the geometryMap between Network and
> > Canvas?
> > The network could offer two methods with a processing key, like this.
> >
> >         Network::AddProcessingGeometry(processing, geometry)
> >         ProcessingGeometry = Network::GetProcessingGeometry(processing)
> > Wouldn't this be simpler (in terms of interface and avoiding coupling)?
> > But maybe I'am missing something. Natanael, David, please go on with
> > your opinions.
>
> I would like the setter and moreover it it was
> AddProcessingGeometry(name,x,y,w,h) as we remove the coupling with a
> network structure, but the getter is the one that clears the geometry if we
> do getter call in a per processing basis we would need a explicit clear
> call. Also that way of doing will ask for every processing even if you
> don't have geometry for it (transition period, see previous comment) so you
> don't have a way to know whether (0,0,0,0) was a real geometry or just it
> hadnt geometry at all.
>
> Loading and resaving networks seems a good transition procedure to me.



-- 
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/20080613/29fbe3a0/attachment.sig>