[Clam-devel] subnetworks interfacing - NE several opened files
Pau Arumí
parumi at iua.upf.edu
Wed Jul 30 02:09:02 PDT 2008
On dc, 2008-07-30 at 03:48 -0300, Natanael Olaiz wrote:
> Here is almost the same refactoring, but BaseNetwork is all pure
> virtual, FlattenedNetwork is the actual Network, and MainWindow uses
> _flattenedNetwork for all player/flow management and BaseNetwork (for
> now the same FlattenedNetwork) to store/load and set the canvas network.
> Plus I changed all the instances of CLAM::Network to FlattenedNetwork
> (but the tests). As I said in my previous mail, that need further
> revision to see what class use in each case, but I think it could serve
> to found easily all the actual uses with the new Network class.
I couldn't apply the patch because of conflicts with last David's
changes in NE MainWindow -- as you said.
Now I'm evaluating qt4.4 backpòrt for hardy...
Anyway, some feedback on this patch:
- Changing all instances of Network to FlattenedNetwork is wrong. The
goal is to change the current (flat) network implementation for a new
(composite) network. *And* the client classes should not be aware of it.
This transition can be easily managed with a Network.hxx header that
contains "typedef FlattenedNetwork Network"
I think that correcting this, the patch is commitable. Some advice:
- Proceed commiting the BaseNetwork files. It is very inconvenient
working with patches containing new files (I want to see the local
changes with svn stat -q)
- I guess you are aware of it. Just a tip for when you commit: when
copying svn files never do "svn add" but "svn cp" or "svn mv" so it
keeps the history
- Try to do small commits. Smaller that this patch. For example, the
decoupling of NE and Network through BaseNetwork (before introducing
FlattenedNetwork) should ideally be one commit
Also, remember the planning. Let's refactor the Network interface (graph
"getter", update changes to flowcontrol) *before* introducing the new
Network class.
Alas, David broke svn version for non qt4.4 users (like us) so let's
hold the commits till we fix this.
P
> BTW, I'd lost a lot of time searching a compilation error which was
> solved with a scons --clear on the library (it was keep searching
> CLAM::Network methods).
>
> And I used r11754. See the reason in my next mail...
>
> I'll start filling up the new Network class...
>
>
> Regards,
> Natanael.
>
> El 07/29/2008 05:47 AM, Pau Arumí escribió:
> > On dt, 2008-07-29 at 04:03 -0300, Natanael Olaiz wrote:
> >
> >> Sending a first attempt of using BaseNetwork abstract to decouple NE
> >> from Network.
> >> It still needs clean-up, there is no FlattenedNetwork yet, neither
> >> subnetworks or changes in the XML methods.... But CLAM::BaseNetwork here
> >> doesn't use/have any flow/player related method, and is used on
> >> ClamNetworkCanvas instead of CLAM::Network.
> >>
> >> BTW: almost all methods on BaseNetwork are virtual (some could be changed).
> >>
> >
> > Well, Some methods in BaseNetwork are virtual pure (=0) but, at the same
> > time, they are implemented. No sense. (I don't understand why the
> > compiler does not complain)
> >
> > What I see is that you've moved stuff from Network to BaseNetwork. No,
> > the idea is maintain all the implementation in Network and create a new
> > "interface" class BaseNetwork. By interface I mean a class made of
> > abstract (virtual pure) methods. No attributes, no code.
> >
> > Making NE to be coupled only to the interface class is the first step.
> > Then we can substitute the FlattenedNetwork (the current Network class)
> > to the new Network class. This new class will have code. It will
> > actually maintain a hierarchical representation of the graph, whose
> > leafs are ID strings (ex: Network/Subnetwork/Processing) that relates
> > the the actual processing in the FlattenedNetwork.
> >
> > Smaller things:
> > In MainWindow.hxx you are still using Network while should be
> > BaseNetwork. But maybe it is just that you wanted to concentrate on
> > changing object for pointer first...
> > Then, about the change "Network _network" to "BaseNetwork * _network" in
> > MainWindow. It is ok, but since the object ownership is still in
> > MainWindow, use a normal attribute instead of a dynamic object (created
> > with new). I mean: "_network = & _flattenedNetwork". This is simpler
> > because we don't have to worry about freeing memory.
> >
> > Waiting for a new patch. Its a good start though :-)
> >
> > P
> >
> >
> >
> >> Next discussed IRC steps:
> >> 1-
> >>
> >> * Rename Network to FlattenedNetwork
> >> * Improve the "graph getter" interface (used by Canvas and FlowControl)
> >> * Refactor FLowControl so it have a unique GraphChanged() method
> >>
> >> 2-
> >>
> >> * Create a new class Network and duplicate the graph model, using
> >> IDs that refers to the flattened network. Eacy graph change is
> >> automatically sync with the flattened network.
> >>
> >> 3
> >>
> >> * introduce subnetworks with a Composite (truly Composite) pattern.
> >> Thoroughly unit tested
> >>
> >> 4-
> >>
> >> * User Interface, and complex workflows (like create subnetwork)
> >>
> >>
> >> I'll continuing on the next steps, but meanwhile we could test and
> >> define the BaseNetwork interface.
> >>
> >>
> >> Regards,
> >> Natanael.
> >>
> >> El 07/24/2008 05:51 AM, Pau Arumí escribió:
> >>
> >>> On dc, 2008-07-23 at 23:06 -0300, Natanael Olaiz wrote:
> >>>
> >>>
> >>>> El 07/23/2008 06:42 PM, Pau Arumí escribió:
> >>>>
> >>>>
> >>>>> On dc, 2008-07-23 at 17:58 -0300, Natanael Olaiz wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>> So, with all this context I ask: what do you think to add the Window
> >>>>>> menu item, and a "close network" subitem on "file", to allow having
> >>>>>> severals opened networks? The active one will be the one you are
> >>>>>> seeing (on the original "Network" tab).
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>> And what would you do with the NetworkPlayer if the user change tabs
> >>>>> while running?
> >>>>>
> >>>>>
> >>>>>
> >>>> Ignore the requests? :)
> >>>>
> >>>> I don't know what is the best way to manage that. Maybe letting the
> >>>> user to chose the view and the active one, maybe having another tab...
> >>>>
> >>>>
> >>> But now that we have copy-paste we can easily move portions of networks
> >>> from one NE instance to another, I really don't see the need for
> >>> multidocument (other that editing multiple levels of a hierarchy).
> >>> Maybe we should do like inkscape (yes, I love that app!): File->Open
> >>> starts a new app instance instead of changing the content of the
> >>> existing one.
> >>>
> >>>
> >>>
> >>>> Hernán had suggested something similar:
> >>>> https://llistes.projectes.lafarga.cat/pipermail/clam-devel/2008/001875.html
> >>>>
> >>>>
> >>>>
> >>>>> I don't see multi-document useful in NetworkEditor (though I'm always
> >>>>> open to get convinced)
> >>>>>
> >>>>> I think that NE with subnetworks should look like a code debugger: you
> >>>>> deal with two views: 1. the stack showing a list higher contexts of the
> >>>>> current inner executing line and 2. the code in the chosen context.
> >>>>>
> >>>>> Now, back to NE: of course 2. corresponds to the network selected in 1.
> >>>>> And 1, could be a textual list, as a first version, and a "list" of
> >>>>> networks miniatures as a second version.
> >>>>>
> >>>>>
> >>>>>
> >>>> I imagined a main running-capable network canvas, from where you can
> >>>> make/open a subnetwork (in the same, or other tab), and if from the
> >>>> new canvas view you can do the same, and so on with
> >>>> sub-sub-...-networks.
> >>>>
> >>>>
> >>>>
> >>>>> However, in my opinion subnetworks should not be approached from the UI,
> >>>>> but from the model. Because now it's hard to decide how will be the
> >>>>> model the UI will interact till it's not done.
> >>>>>
> >>>>>
> >>>>>
> >>>> Yes. That is the reason because I said to duplicate some parameters
> >>>> for now, on the main canvas. But I though the graphical management
> >>>> (canvas) as a useful tool to test and see the models. Anyway, now I
> >>>> understand better your idea on the reply to Hernán in the previous
> >>>> thread.
> >>>>
> >>>>
> >>>>
> >>>>> Refreshing a talk at irc, this should be the steps:
> >>>>> 1. improve current Network interface to facilitate Network clients
> >>>>> (basically NetworkCanvas and FlowControl) getting the graph
> >>>>> 2. make Network a composite (as in the pattern).
> >>>>> 3. work ou the UI
> >>>>>
> >>>>>
> >>>>>
> >>>> But on 1. I used methods of ClamNetworkCanvas and inherited members.
> >>>> For instance, how would you make the subnetworks? I started with
> >>>> selecting the processings on the canvas, and reusing the actual
> >>>> network xml dump and restore methods (used for loading/saving and
> >>>> copy&paste). Another option I think could be to import a network as a
> >>>> subnetwork. But the positions are managed from ClamNetworkCanvas, not
> >>>> NetworkCanvas. Should I ignore the geometries for now, or implement
> >>>> those methods on NetworkCanvas as a part of the refactoring and let
> >>>> all the CLAM::Networks management be just on NetworkCanvas?
> >>>>
> >>>>
> >>> It is interesting to analyze how subnetworks will be created, from the
> >>> user point of view. I can think of this:
> >>> 1. the user starts a new subnetwork from scratch, and then
> >>> populates it.
> >>> 2. the user adds a subnetwork choosing from a network file (or a
> >>> network-tree, similar or merged with processing-tree.
> >>> 3. the user select a subset of a network and hits a "extract to
> >>> subnetwork" button
> >>> 4. the user select a subset of a network and hits a "save as
> >>> network file" (we might want to have the saved network in the
> >>> network-tree)
> >>>
> >>> 2,3 and 4 are basically 1 doing some "extra work". I'm not sure if this
> >>> "extra work" should be responsibility of the NetworkCanvas or the
> >>> Network (I would say the first).
> >>> So we should begin addressing the interface (and tests) to Network in
> >>> order to do 1. For example:
> >>>
> >>> Network subnet;
> >>> subnet.AddProcessing(..);
> >>> ...
> >>> Network net;
> >>> net.AddNetwork( subnet );
> >>>
> >>> Or maybe we should generalize Processing/Network -> Module? so
> >>> net.AddModule
> >>>
> >>>
> >>> About the NCanvas and Network relation:
> >>> I think we should have NCanvas instances only for (sub)networks that are
> >>> currently opened --so, not for the whole hierarchy because this would be
> >>> like duplicating the model, making difficult to add and remove subtrees.
> >>> Therefore the position information should reside in the Network object
> >>> (we already have this), that means that each time a canvas is closed it
> >>> must synchronize its position info to its network.
> >>>
> >>> P
> >>>
> >>>
> >>>
> >>>
> >>>>> Point 2 is difficult. My design proposal (discussed with David) is the
> >>>>> following: the root of the composite should have a FlattenedNetwork
> >>>>> object associated. FlattenedNetwork and Network both derive from an
> >>>>> abstract BaseNetwork. Only FlattenedNetwork have a NetworkPlayer and a
> >>>>> FlowControl, and owns all the processings in the hierarchy.
> >>>>> This should be more discussed, and developed test-driven in small steps.
> >>>>>
> >>>>>
> >>>>>
> >>>> OK.
> >>>>
> >>>>
> >>>> Regards,
> >>>> Natanael.
> >>>>
> >>>>
> >>>>> P
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> Clam-devel mailing list
> >>>>> Clam-devel at llistes.projectes.lafarga.org
> >>>>> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>> _______________________________________________
> >>>> Clam-devel mailing list
> >>>> Clam-devel at llistes.projectes.lafarga.org
> >>>> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >>>>
> >>>>
> >>> _______________________________________________
> >>> Clam-devel mailing list
> >>> Clam-devel at llistes.projectes.lafarga.org
> >>> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >>>
> >>>
> >>>
> >> _______________________________________________
> >> Clam-devel mailing list
> >> Clam-devel at llistes.projectes.lafarga.org
> >> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >>
> >
> >
> > _______________________________________________
> > Clam-devel mailing list
> > Clam-devel at llistes.projectes.lafarga.org
> > https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >
> >
>
> _______________________________________________
> Clam-devel mailing list
> Clam-devel at llistes.projectes.lafarga.org
> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
More information about the clam-devel
mailing list