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

David Tardon dtardon at redhat.com
Thu Jan 6 03:44:43 PST 2011


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.


More information about the LibreOffice mailing list