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

List:       kde-commits
Subject:    [marble] /: always create an empty bookmark document upon construction
From:       Bernhard Beschow <bbeschow () cs ! tu-berlin ! de>
Date:       2012-10-31 19:40:34
Message-ID: 20121031194034.AE74BA60D4 () git ! kde ! org
[Download RAW message or body]

Git commit 9e630ca10ce4fa175a63dfc7a4e9cda2b73cba4f by Bernhard Beschow.
Committed on 31/10/2012 at 17:53.
Pushed by beschow into branch 'master'.

always create an empty bookmark document upon construction

Establishing the invariant that a document is always around avoids null pointer \
issues. In addition, creating an empty document upon construction should be \
indistinguishable from outside (compared to the previous approach where a document is \
created on demand).

BUG: 309273

M  +4    -12   src/lib/BookmarkManager.cpp
M  +1    -1    src/lib/BookmarkManagerDialog.cpp
M  +0    -2    src/lib/BookmarkManager_p.h
M  +8    -25   tests/BookmarkManagerTest.cpp

http://commits.kde.org/marble/9e630ca10ce4fa175a63dfc7a4e9cda2b73cba4f

diff --git a/src/lib/BookmarkManager.cpp b/src/lib/BookmarkManager.cpp
index d77d64a..4fd4e8b 100644
--- a/src/lib/BookmarkManager.cpp
+++ b/src/lib/BookmarkManager.cpp
@@ -31,7 +31,7 @@ BookmarkManagerPrivate::BookmarkManagerPrivate( GeoDataTreeModel \
*treeModel ) :  m_bookmarkDocument( 0 ),
     m_bookmarkFileRelativePath( "bookmarks/bookmarks.kml" )
 {
-    // nothing to do
+    resetBookmarkDocument();
 }
 
 BookmarkManagerPrivate::~BookmarkManagerPrivate()
@@ -57,14 +57,6 @@ void BookmarkManagerPrivate::resetBookmarkDocument()
     m_treeModel->addDocument( m_bookmarkDocument );
 }
 
-GeoDataDocument* BookmarkManagerPrivate::bookmarkDocument()
-{
-    if ( !m_bookmarkDocument ) {
-        resetBookmarkDocument();
-    }
-    return m_bookmarkDocument;
-}
-
 void BookmarkManagerPrivate::setVisualCategory( GeoDataContainer *container ) {
     foreach( GeoDataFolder* folder, container->folderList() ) {
         setVisualCategory( folder );
@@ -154,7 +146,7 @@ void BookmarkManager::removeBookmark( GeoDataPlacemark *bookmark \
)  
 GeoDataDocument * BookmarkManager::document() const
 {
-    return d->bookmarkDocument();
+    return d->m_bookmarkDocument;
 }
 
 bool BookmarkManager::showBookmarks() const
@@ -170,7 +162,7 @@ void BookmarkManager::setShowBookmarks( bool visible )
 
 QVector<GeoDataFolder*> BookmarkManager::folders() const
 {
-    return d->bookmarkDocument()->folderList();
+    return d->m_bookmarkDocument->folderList();
 }
 
 void BookmarkManager::addNewBookmarkFolder( GeoDataContainer *container, const \
QString &name ) @@ -240,7 +232,7 @@ bool BookmarkManager::updateBookmarkFile()
 
         file.open( QIODevice::WriteOnly );
 
-        if ( !writer.write( &file,  d->bookmarkDocument() ) ) {
+        if ( !writer.write( &file, d->m_bookmarkDocument ) ) {
             mDebug() << "Could not write the bookmarks file" << \
absoluteLocalFilePath;  file.close();
             return false;
diff --git a/src/lib/BookmarkManagerDialog.cpp b/src/lib/BookmarkManagerDialog.cpp
index 2dcb7cf..41648bd 100644
--- a/src/lib/BookmarkManagerDialog.cpp
+++ b/src/lib/BookmarkManagerDialog.cpp
@@ -504,7 +504,7 @@ void BookmarkManagerDialog::importBookmarks()
 
 GeoDataDocument* BookmarkManagerDialog::bookmarkDocument()
 {
-    return d->m_manager->d->bookmarkDocument();
+    return d->m_manager->document();
 }
 
 void BookmarkManagerDialog::setButtonBoxVisible( bool visible )
diff --git a/src/lib/BookmarkManager_p.h b/src/lib/BookmarkManager_p.h
index 5598218..4fc3b95 100644
--- a/src/lib/BookmarkManager_p.h
+++ b/src/lib/BookmarkManager_p.h
@@ -27,8 +27,6 @@ public:
 
     ~BookmarkManagerPrivate();
 
-    GeoDataDocument* bookmarkDocument();
-
     void resetBookmarkDocument();
 
     static void setVisualCategory( GeoDataContainer *container );
diff --git a/tests/BookmarkManagerTest.cpp b/tests/BookmarkManagerTest.cpp
index 9d78b77..b821d75 100644
--- a/tests/BookmarkManagerTest.cpp
+++ b/tests/BookmarkManagerTest.cpp
@@ -40,36 +40,20 @@ void BookmarkManagerTest::construct()
     {
         const BookmarkManager manager( &model );
 
-        QCOMPARE( model.rowCount(), 0 );
-
-        QVERIFY( manager.document() != 0 );
-
         QCOMPARE( model.rowCount(), 1 );
-    }
-
-    QCOMPARE( model.rowCount(), 0 );
-
-    {
-        const BookmarkManager manager( &model );
-
-        QCOMPARE( model.rowCount(), 0 );
 
+        QVERIFY( manager.document() != 0 );
         QCOMPARE( manager.folders().count(), 1 );
         QCOMPARE( manager.folders().first()->size(), 0 );
+        QCOMPARE( manager.showBookmarks(), true );
+
+        // FIXME this method returns random results (depending on the username and \
the existance of the bookmarks file) +        //    QCOMPARE( manager.bookmarkFile(), \
QString() );  
         QCOMPARE( model.rowCount(), 1 );
     }
 
     QCOMPARE( model.rowCount(), 0 );
-
-// FIXME this test case crashes because BookmarkManagerPrivate::m_bookmarkDocument \
                == 0
-#if 0
-    {
-        const BookmarkManager manager( &model );
-
-        QCOMPARE( manager.showBookmarks(), true );
-    }
-#endif
 }
 
 void BookmarkManagerTest::loadFile_data()
@@ -92,15 +76,14 @@ void BookmarkManagerTest::loadFile()
     GeoDataTreeModel model;
     BookmarkManager manager( &model );
 
-    QVERIFY( model.rowCount() == 0 );
+    QVERIFY( model.rowCount() == 1 );
 
     const bool fileLoaded = manager.loadFile( relativePath );
 
     QCOMPARE( fileLoaded, expected );
+    QVERIFY( manager.document() != 0 );
 
-    const int rowCount = fileLoaded ? 1 : 0;
-
-    QCOMPARE( model.rowCount(), rowCount );
+    QCOMPARE( model.rowCount(), 1 );
 }
 
 }


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