[Clam-devel] subnetworks interfacing - NE several opened files
Natanael Olaiz
nolaiz at gmail.com
Wed Jul 30 02:46:53 PDT 2008
El 07/30/2008 06:09 AM, Pau Arumí escribió:
> 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...
>
I compiled it and did a "make install" (on /usr/local/Trolltech).
Setting the QTDIR and PATH to the /usr/local/Trolltech/.... backed up
the previous dirs /usr/include/qt4 and /usr/lib/qt4 and linked to the
correspondent dirs on /usr/local/Trolltech was enough to compile, but
not to run. :-/
If you install it successfully, please tell us. ;-)
> 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"
>
Yes, for the commit. But as I said, I wanted to know which are the
clients, to review the uses with the refactored classes.
> 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
>
Oops, I did not aware! (just added FlattenedNetwork, which is the actual
Network...). So, great advice. :)
> - 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
>
OK.
> Also, remember the planning. Let's refactor the Network interface (graph
> "getter", update changes to flowcontrol) *before* introducing the new
> Network class.
>
With graph getter do you mean graph as a networks hierarchy tree? The
implementation of that would not go on the newer Network class?
> Alas, David broke svn version for non qt4.4 users (like us) so let's
> hold the commits till we fix this.
>
OK.
Regards,
Natanael.
> 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
>>
>
>
> _______________________________________________
> 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