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

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.