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

List:       kde-commits
Subject:    Re: KDE/kdepimlibs/akonadi
From:       Bertjan Broeksema <bertjan () kdab ! com>
Date:       2010-01-18 9:00:57
Message-ID: 201001181000.57665.bertjan () kdab ! com
[Download RAW message or body]

On Friday 15 January 2010 20:37:44 Tom Albers wrote:
> SVN commit 1075299 by toma:
> 
> Don't use var names which are also member vars or used by parent classes.
> 
> 
>  M  +3 -3      collection_p.h
>  M  +3 -3      collectionsync.cpp
>  M  +2 -2      collectionview.cpp
>  M  +3 -3      monitor_p.cpp
> 
> 
> --- trunk/KDE/kdepimlibs/akonadi/collection_p.h #1075298:1075299
> @@ -71,11 +71,11 @@
> 
>      static Collection newRoot()
>      {
> -      Collection root( 0 );
> +      Collection rootCollection( 0 );
>        QStringList types;
>        types << Collection::mimeType();
> -      root.setContentMimeTypes( types );
> -      return root;
> +      rootCollection.setContentMimeTypes( types );
> +      return rootCollection;
>      }

A global static function doesn't have a parent class, so this change is quite 
pointless right?! Was there a warning for this function?

>      QString name;
> --- trunk/KDE/kdepimlibs/akonadi/collectionsync.cpp #1075298:1075299
> @@ -243,10 +243,10 @@
>        to see if any of them can be processed by now. If not, they are
>  moved to the closest ancestor available.
>      */
> -    void processPendingRemoteNodes( LocalNode *localRoot )
> +    void processPendingRemoteNodes( LocalNode *_localRoot )
>      {
> -      QList<RemoteNode*> pendingRemoteNodes( localRoot->pendingRemoteNodes
>  ); -      localRoot->pendingRemoteNodes.clear();
> +      QList<RemoteNode*> pendingRemoteNodes(
>  _localRoot->pendingRemoteNodes ); +     
>  _localRoot->pendingRemoteNodes.clear();
>        QHash<LocalNode*, QList<RemoteNode*> > pendingCreations;
>        foreach ( RemoteNode *remoteNode, pendingRemoteNodes ) {
>          // step 1: see if we have a matching local node already
> --- trunk/KDE/kdepimlibs/akonadi/collectionview.cpp #1075298:1075299
> @@ -179,8 +179,8 @@
> 
>    // Check if the collection under the cursor accepts this data type
>    const QStringList supportedContentTypes = model()->data( index,
>  CollectionModel::CollectionRole ).value<Collection>().contentMimeTypes();
>  -  const QMimeData *data = event->mimeData();
> -  const KUrl::List urls = KUrl::List::fromMimeData( data );
> +  const QMimeData *mimeData = event->mimeData();
> +  const KUrl::List urls = KUrl::List::fromMimeData( mimeData );
>    foreach( const KUrl &url, urls ) {
> 
>      const Collection collection = Collection::fromUrl( url );
> --- trunk/KDE/kdepimlibs/akonadi/monitor_p.cpp #1075298:1075299
> @@ -223,9 +223,9 @@
> 
>  void MonitorPrivate::slotSessionDestroyed( QObject * object )
>  {
> -  Session* session = qobject_cast<Session*>( object );
> -  if ( session )
> -    sessions.removeAll( session->sessionId() );
> +  Session* objectSession = qobject_cast<Session*>( object );
> +  if ( objectSession )
> +    sessions.removeAll( objectSession->sessionId() );
>  }
> 
>  void MonitorPrivate::slotStatisticsChangedFinished( KJob* job )

I'd be more in favor of renaming the members by prepending m or m_ (whatever 
the standard is in kdepimlibs. This makes it more visible when using members 
in stead of local variables and prevents these kind of warnings.

Cheers,

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