On Wed, Jan 05, 2011 at 11:05:02AM +0000, Noel Power wrote: > 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(); > }
Another nitpick: std::vector::at (as opposed to std::vector::operator[]) checks its argument and throws an exception if it is out of bounds. This check is absolutely useless inside a loop (well, unless the end condition is wrong) and only adds to run time. D. _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
