[Clam-devel] Request for votes: Thinking on 'Connect to' feature

Natanael Olaiz nolaiz at gmail.com
Thu Jun 19 12:12:20 PDT 2008


El 06/19/2008 08:47 AM, Pau Arumí Albó escribió:
> El dj 19 de 06 de 2008 a les 06:51 -0300, en/na Natanael Olaiz va
> escriure:
>   
>> Are you hungry? Here you have some spaghetti.... :-/
>>
>> It's a really ugly code, but now it really does what you purposed. :)
>> Just for test if you like the interface...
>> It NEED to merge and refactor code on the context menu... and it would
>> be good to merge also some code between onProcessingsConnectTo() and
>> onAddLinkedProcessing(). For instance, to use explicits "Add xxxxx" on
>> context menu titles, I'm using regexp to cut the string within
>> onAddLinkedProcessing() and get the processing key. 
>>     
>> Do you like the QMap <QString, QVariant> method used on
>> onProcessingsConnectTo() to use in AddLinked... too?
>>     
>
> Yes, I think that instead of parsing the text with regexs (which btw is
> more subjected to change) it is better to pass a QMap with the needed
> arguments, like in other Actions
>
> BTW, naming convention: s/typeConnectionMap/ConnectionMap
>   
> I'm reading the onAddLinkedProcessing() code and I don't understand
> why/if it works. It seems to me that it is trying to add a new linked
> processing to all existing processings.
> I guess the trick is in the delegated method
> (addLinkedProcessingSender/Receiver): 
> unsigned portIndex = processing->portIndexByYPos(point);
> this might return an invalid port index. But I miss the conditional code
> that avoids adding a processing box on invalid cases.
>
> I think that is David code. A clue, anyone? (I'm lazy to debug right
> now)
>   
onAddLinkedProcessing() checks for ProcessingBox::inportsRegion or 
ProcessingBox::outportsRegion to call 
addLinkedProcessingSender/Receiver, so portIndexByYPos gives the index 
of that port, which is on the passed ProcessingBox *.

> All the In/OutPort/Controls connections stuff deals with a lot of
> variations. This is not easy! I like your solution of collecting the
> proper member functions (nTargetConnections, etc) to have reusable code.
> However, it smells like we could redesign in order to manage all the
> connection types in a (more) homogeneous way. But not for now.
>   

I agree. Maybe reusing that code on some helpers...

> Yes, keep commiting your local changes, and refactoring where you see
> the opportunity.
>   

OK. But, this patch too? It's not buggy, but ugly and unoptimized...

> P
>
>   
>> Regards,
>> Natanael.
>>
>> El 06/18/2008 04:14 AM, Pau Arumí Albó escribió: 
>>     
>>> El dt 17 de 06 de 2008 a les 20:05 -0300, en/na Natanael Olaiz va
>>> escriure:
>>>
>>>   
>>>       
>>>> Here is the same patch without the network geometries refactoring. Is 
>>>> OK? If you agree, please commit it and then I'll send the refactoring 
>>>> network geometries patch onto the new revision to discuss it.
>>>>     
>>>>         
>>> Go ahead, check this one in yourself.
>>>
>>>
>>>   
>>>       
>
>
>   





More information about the clam-devel mailing list