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

[Pau Arumí Albó]

El dv 13 de 06 de 2008 a les 13:38 +0200, en/na David García Garzón va
escriure:
> 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.

OK -- David is the CLAM's xml master :-) 
> 
> > > 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.

Yes -- and I like being explicit :)

>  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.

Easy, let (*,*,0,0) be a non-existing geometry (we don't want invisible
boxes).


P