Re: [Libreoffice] [PATCH] Remove DECLARE_LIST( EditList, AppWIn*)

Hi Joseph
On -10/01/37 20:59, Joseph Powers wrote:
> The attached patch compiles and stands up to my limited testing; however, it's a large patch and 
> touches a  lot of sensitive code so I want someone with better knowledge of the Basic Macro Editor  
> environment to review&  test it before I try pushing it.
>
> Thanks,
> Joe P.
I wouldn't claim to have much knowledge of the Basic Macro editor but I 
suppose at least I have touched it in the past ;-)
Firstly congratulations on this is great work, getting rid of those 
horrific macro lists and replacing with something more modern surely 
will make things easier for new developers to understand.  As far as I 
can see the changes as they stand shouldn't cause any problems and could 
be committed.
My only comment would be that mostly  the opportunity to simplify the 
code using the power of the vector has not being taken advantage of 
which is a pity ( and would be great to fix here ) e.g.

@@ -627,17 +627,15 @@ void BasicFrame::LoadIniFile()
     if ( pBasic )
         pBasic->LoadIniFile();

-    for ( i = 0 ; i < pList->Count() ; i++ )
-        pList->GetObject( i )->LoadIniFile();
+    for ( i = 0 ; i < pList->size() ; i++ )
+        pList->at( i )->LoadIniFile();
 }

could be much clearly expressed using iterators ( and no need to check 
the size every loop iteration either )
e.g. something like

EditList::iterator it = pList->end()
for ( EditList::iterator it = pList->begin(); it != it_end; ++it )
    *it->LoadIniFile();

similarly really ugly index manipulation e.g.

@@ -777,11 +775,10 @@ void BasicFrame::Resize()


     // Resize possibly maximized window
-    ULONG i;
-    for( i = pList->Count(); i > 0 ; i-- )
+    for( size_t i = pList->size(); i > 0 ; i-- )
     {
-        if ( pList->GetObject( i-1 )->GetWinState() == TT_WIN_STATE_MAX )
-            pList->GetObject( i-1 )->Maximize();
+        if ( pList->at( i-1 )->GetWinState() == TT_WIN_STATE_MAX )
+            pList->at( i-1 )->Maximize();
     }
 }

could be simplified and avoided by again making use of the power of the 
vector

    EditList::iterator::reverse_iterator rit_end = pList->rend();
    for ( EditList::iterator::reverse_iterator rit=pList->rbegin() ; 
rit != rit_end; ++rit )
    {
        if ( *rit->GetWinState() == TT_WIN_STATE_MAX )
            *rit->Maximize();
    }

nearly all of the hunks in the patch could be modified similarly I think.
note: my crappy pseudo code bits above are not compiled or tested but 
you get what I mean I am sure

thanks again

Noel