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

Pau Arumí Albó parumi at iua.upf.edu
Fri Jun 13 02:39:57 PDT 2008


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!

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


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.

(If finally agree, we could as well live this as a posterior
refactoring, after commit)


P






More information about the clam-devel mailing list