Conversation
|
Hi @AndriiVoitenko, |
adragnevVW
requested review from
AndriiVoitenko
and removed request for
AndriiVoitenko
|
Hi @AndriiVoitenko , |
|
@adragnevVW I’m not quite sure I understand the purpose of this optimization. The problem is that several different object types are combined into one fAuxDataObjs. That means that each API returning a specific object type at a given position have to search through the shared list. If I understand correctly, you created a parallel list fSymDefObjs that contains only SymDef objects to avoid searching by index. But what about the other object types? Why didn’t you create separate lists for them as well? With this changes you optimize only API for SymDef. And why you leave them in a general list? If SymDef objects already have they own dedicated list, why do we need the shared one? |
| } | ||
|
|
||
| // Increase position count | ||
| positionCount++; |
There was a problem hiding this comment.
Without incrementing of posionCount will be endless loop, or I oversee something?
| newSymDef->ReadFromNode(node, this); | ||
|
|
||
| fAuxDataObjs.push_back(newSymDef); | ||
| fSymDefObjs.push_back( newSymDef ); |
There was a problem hiding this comment.
newSymDef comes in both fAuxDataObjs and fSymDefObjs. Are you sure?
ILane-VW
left a comment
ILane-VW
left a comment
There was a problem hiding this comment.
see changes from Andrii
3 participants
Fix for #123:
Changes:
Improving GetSymDefAt() and GetSymDefCount()
Adding a check if the symbol contains other symbol as a child.