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
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to