[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: kdelibs4todo # 58
From:       Frans Englich <englich () kde ! org>
Date:       2006-04-25 21:58:56
Message-ID: 200604252158.57020.englich () kde ! org
[Download RAW message or body]

On Tuesday 25 April 2006 21:39, Michael Pyne wrote:
> On Tuesday 25 April 2006 08:20, Will Entriken wrote:
> > #58 Places where things like "foo = new QString" seem like prime
> > candidates for refactoring.
> >
> > I found all the cases of this using grep -nI "new QString\b" -R * |
> > grep -v 'svn' and I got:
> >
> > kdecore/kicontheme.cpp:426:    _theme = new QString;
> > kdecore/kglobalsettings.cpp:481:    s_desktopPath = new QString();
> > kdecore/kglobalsettings.cpp:482:    s_autostartPath = new QString();
> > kdecore/kglobalsettings.cpp:483:    s_documentPath = new QString();
> > khtml/khtml_settings.cc:884:        avFamilies = new QString;
> > khtml/test_regression_fontoverload.cpp:495:        avFamilies = new
> > QString; kio/kio/netaccess.cpp:62:            lastErrorMsg = new QString;
> > kio/kio/netaccess.cpp:434:      lastErrorMsg = new QString;
> >
> > What is the strategy to refactor these?
>
> To be honest I'm not sure why this is a TODO.  Is there some problem with
> having pointers to QString, aside from the fact that it is probably not
> necessary to use pointers to QString?

In general it's very bad practice since value based programming is much safer 
than pointer based. A pointer can be null, can dangle, utter confusion about 
ownership, leaks, etc, etc.

So, if it's part of an API it's of especial high interest to get rid of it 
during KDE 4 development, IMHO.

However, in some of the above cases it looks like static data is used, which I 
guess could justify pointers to QString. Though, I think Qt got fancy 
alternatives for that; the Q_STATIC* macros. Not that I know much about 
KGlobalSettings, someone surely got a competent reply for that.


Cheers,

		Frans
 
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic