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

[David García Garzón]

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: not available
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/654127ff/attachment-0003.pgp>