[Liblas-commits] hg: Fixed consistent use of integer types. Added number of asser...

liblas-commits at liblas.org liblas-commits at liblas.org
Mon Oct 19 19:25:04 EDT 2009


changeset d44e9250826f in /home/www/liblas.org/hg
details: http://hg.liblas.org/main/hg?cmd=changeset;node=d44e9250826f
summary: Fixed consistent use of integer types. Added number of assertions. Pointed out lack or RAII and exception safety. Moved private elements at the bottom of class definitions. Added missing headers. Cleaned number of compile-time warnings.

diffstat:

 include/liblas/index/query.hpp |  46 ++++++++++++-----------
 src/index/index.cpp            |   1 +
 src/index/query.cpp            |  72 +++++++++++++++++-------------------
 src/index/storage.cpp          |   7 +--
 src/index/visitor.cpp          |  18 +++++---
 5 files changed, 73 insertions(+), 71 deletions(-)

diffs (truncated from 304 to 300 lines):

diff -r c402c23bd5aa -r d44e9250826f include/liblas/index/query.hpp
--- a/include/liblas/index/query.hpp	Tue Oct 20 00:03:20 2009 +0100
+++ b/include/liblas/index/query.hpp	Tue Oct 20 00:24:38 2009 +0100
@@ -50,6 +50,7 @@
 #endif
 
 //std
+#include <list>
 #include <string>
 #include <vector>
 #include <stack>
@@ -60,14 +61,9 @@
 
 class LASQueryResult 
 {
-private:
-    std::list<SpatialIndex::id_type> ids;
-    SpatialIndex::Region* bounds;
-    uint32_t m_id;
-    LASQueryResult();
 public:
-    LASQueryResult(uint32_t id) : bounds(0), m_id(id){};
-    ~LASQueryResult() {if (bounds!=0) delete bounds;};
+    LASQueryResult(uint32_t id) : bounds(0), m_id(id) {}
+    ~LASQueryResult() { delete bounds; } // FIXME: wrap bounds with smart pointer
 
     /// Copy constructor.
     LASQueryResult(LASQueryResult const& other);
@@ -76,35 +72,41 @@
     LASQueryResult& operator=(LASQueryResult const& rhs);
         
     std::list<SpatialIndex::id_type> const& GetIDs() const;
-    void SetIDs(std::list<SpatialIndex::id_type>& v);
+    void SetIDs(std::list<SpatialIndex::id_type>& v); // FIXME: Why not const-reference?
     const SpatialIndex::Region* GetBounds() const;
-    void SetBounds(const SpatialIndex::Region*  b);
+    void SetBounds(const SpatialIndex::Region* b);
     uint32_t GetID() const {return m_id;}
     void SetID(uint32_t v) {m_id = v;}
+
+private:
+    std::list<SpatialIndex::id_type> ids;
+    SpatialIndex::Region* bounds;
+    uint32_t m_id;
+    LASQueryResult();
 };
 
 class LASQuery : public SpatialIndex::IQueryStrategy
 {
+public:
+
+    LASQuery();
+    ~LASQuery()
+    {
+        std::cout << "child count was" << m_count << std::endl;
+        std::cout << "results count was" << m_results.size() << std::endl;
+    }
+    void getNextEntry(const SpatialIndex::IEntry& entry, SpatialIndex::id_type& nextEntry, bool& hasNext);
+    
+    std::list<LASQueryResult>& GetResults() {return m_results;}
+    SpatialIndex::Region bounds;
+
 private:
     std::queue<SpatialIndex::id_type> m_ids;
     uint32_t m_count;
     std::list<LASQueryResult> m_results;
     bool m_first;
-    
-public:
-
-    LASQuery();
-    ~LASQuery() {
-        std::cout << "child count was" << m_count << std::endl;
-        std::cout << "results count was" << m_results.size() << std::endl;};
-    void getNextEntry(const SpatialIndex::IEntry& entry, SpatialIndex::id_type& nextEntry, bool& hasNext);
-    
-    std::list<LASQueryResult>& GetResults() {return m_results;}
-    SpatialIndex::Region bounds;
 };
 
-
-
 }
 
 #endif // LIBLAS_INDEX_QUERY_HPP_INCLUDED
diff -r c402c23bd5aa -r d44e9250826f src/index/index.cpp
--- a/src/index/index.cpp	Tue Oct 20 00:03:20 2009 +0100
+++ b/src/index/index.cpp	Tue Oct 20 00:24:38 2009 +0100
@@ -99,6 +99,7 @@
 
 LASIndex::LASIndex(LASIndex const& other) 
 {
+    detail::ignore_unused_variable_warning(other);
     std::cout << "Index copy called" << std::endl;
 }
 
diff -r c402c23bd5aa -r d44e9250826f src/index/query.cpp
--- a/src/index/query.cpp	Tue Oct 20 00:03:20 2009 +0100
+++ b/src/index/query.cpp	Tue Oct 20 00:24:38 2009 +0100
@@ -44,22 +44,23 @@
 
 #include <cstddef>
 #include <iostream>
+#include <list>
 #include <sstream>
 #include <string>
 #include <vector>
 #include <stdexcept>
 
-
 namespace liblas
 {
 
-    LASQuery::LASQuery() :m_count(0),m_first(true){}
+LASQuery::LASQuery() : m_count(0), m_first(true) {}
 
-void LASQuery::getNextEntry(const SpatialIndex::IEntry& entry, SpatialIndex::id_type& nextEntry, bool& hasNext) {
+void LASQuery::getNextEntry(const SpatialIndex::IEntry& entry, SpatialIndex::id_type& nextEntry, bool& hasNext)
+{
     // the first time we are called, entry points to the root.
-    
-    if (m_first) {
-        SpatialIndex::IShape* ps;
+    if (m_first)
+    {
+        SpatialIndex::IShape* ps = 0; // FIXME: No RAII, use smart pointer
         entry.getShape(&ps);
         ps->getMBR(bounds);
         delete ps;
@@ -69,39 +70,34 @@
         m_first = false;
     }
     const SpatialIndex::INode* n = dynamic_cast<const SpatialIndex::INode*>(&entry);
-    
-    // if (n !=0 ) {
-    //     for (size_t i = 0; i < n->getChildrenCount(); i++) 
-    //     {
-    //         m_ids.push(n->getChildIdentifier(i));
-    //     }
-    // }
+    assert(0 != n);
 
-		// traverse only index nodes at levels 2 and higher.
-		if (n != 0 && n->getLevel() > 0)
-		{
-            m_count +=n->getChildrenCount();
-			for (size_t cChild = 0; cChild < n->getChildrenCount(); cChild++)
-			{
-				m_ids.push(n->getChildIdentifier(cChild));
-			}
-		}
-		
-		if (n->isLeaf()) {
+    // traverse only index nodes at levels 2 and higher.
+    if (n != 0 && n->getLevel() > 0)
+    {
+        m_count +=n->getChildrenCount();
+        for (std::size_t cChild = 0; cChild < n->getChildrenCount(); cChild++)
+        {
+            m_ids.push(n->getChildIdentifier(cChild));
+        }
+    }
 
-            SpatialIndex::IShape* ps;
-		    entry.getShape(&ps);
-		    SpatialIndex::Region* pr = dynamic_cast<SpatialIndex::Region*>(ps);
-            std::list<SpatialIndex::id_type> ids;
-			for (size_t cChild = 0; cChild < n->getChildrenCount(); cChild++)
-			{
-				ids.push_back(n->getChildIdentifier(cChild));
-			}
-            LASQueryResult result(n->getIdentifier());
-            result.SetIDs(ids);
-            result.SetBounds(pr);
-            m_results.push_back(result);
-		}
+    if (n->isLeaf())
+    {
+        SpatialIndex::IShape* ps = 0; // FIXME: Use RAII
+        entry.getShape(&ps);
+        SpatialIndex::Region* pr = dynamic_cast<SpatialIndex::Region*>(ps);
+        assert(0 != pr);
+        std::list<SpatialIndex::id_type> ids;
+        for (std::size_t cChild = 0; cChild < n->getChildrenCount(); cChild++)
+        {
+            ids.push_back(n->getChildIdentifier(cChild));
+        }
+        LASQueryResult result(static_cast<uint32_t>(n->getIdentifier()));
+        result.SetIDs(ids);
+        result.SetBounds(pr);
+        m_results.push_back(result);
+    }
 		    
     if (! m_ids.empty())
     {
@@ -140,6 +136,7 @@
 
 void LASQueryResult::SetBounds(const SpatialIndex::Region*  b) 
 {
+    assert(0 != b);
     bounds = new SpatialIndex::Region(*b);
 }
 
@@ -164,5 +161,4 @@
     return *this;
 }
 
-
 } // namespace liblas
diff -r c402c23bd5aa -r d44e9250826f src/index/storage.cpp
--- a/src/index/storage.cpp	Tue Oct 20 00:03:20 2009 +0100
+++ b/src/index/storage.cpp	Tue Oct 20 00:24:38 2009 +0100
@@ -54,7 +54,6 @@
 namespace liblas
 {
 
-
 SpatialIndex::IStorageManager* returnVLRStorageManager(Tools::PropertySet& ps)
 {
     SpatialIndex::IStorageManager* sm = new VLRStorageManager(ps);
@@ -69,6 +68,7 @@
 
 VLRStorageManager::VLRStorageManager(Tools::PropertySet& ps)
 {
+    detail::ignore_unused_variable_warning(ps);
 }
 
 VLRStorageManager::~VLRStorageManager()
@@ -102,7 +102,6 @@
 
 void VLRStorageManager::storeByteArray(SpatialIndex::id_type& id, const std::size_t len, const uint8_t* const data)
 {
-    
     if (id == SpatialIndex::StorageManager::NewPage)
     {
         LASVariableRecord* v = makeVLR(len, data);
@@ -136,7 +135,7 @@
 
         LASVariableRecord* v = makeVLR(len, data);
         assert(0 != v);
-        
+
         delete v_old;
         m_vlrbuffer[static_cast<vlrbuffer_t::size_type>(id)] = v;
     }
@@ -169,7 +168,7 @@
     v->SetRecordLength(static_cast<uint16_t>(len));
     v->SetUserId("liblas.org");
     v->SetRecordId(2112);
-    
+
     typedef std::vector<uint8_t> data_t;
     data_t d;
     for (data_t::size_type i = 0; i < len; ++i)
diff -r c402c23bd5aa -r d44e9250826f src/index/visitor.cpp
--- a/src/index/visitor.cpp	Tue Oct 20 00:03:20 2009 +0100
+++ b/src/index/visitor.cpp	Tue Oct 20 00:24:38 2009 +0100
@@ -54,8 +54,10 @@
 namespace liblas
 {
 
-
-LASVisitor::LASVisitor(std::vector<uint32_t>* vect){m_vector = vect;}
+LASVisitor::LASVisitor(std::vector<uint32_t>* vect) : m_vector(vect)
+{
+    assert(0 != vect);
+}
 
 void LASVisitor::visitNode(const SpatialIndex::INode& n)
 {
@@ -66,9 +68,12 @@
 
 void LASVisitor::visitData(const SpatialIndex::IData& d)
 {
-    SpatialIndex::IShape* pS;
+    SpatialIndex::IShape* pS = 0;
     d.getShape(&pS);
+    assert(0 != pS);
+
     SpatialIndex::Region *r = new SpatialIndex::Region();
+    assert(0 != r);
     pS->getMBR(*r);
     std::cout <<"found shape: " << *r << " dimension: " <<pS->getDimension() << std::endl;
         // do something.
@@ -77,16 +82,15 @@
 
     // data should be an array of characters representing a Region as a string.
     uint8_t* pData = 0;
-    ::uint32_t cLen = 0;
+    std::size_t cLen = 0;
     d.getData(cLen, &pData);
     // do something.
     //string s = reinterpret_cast<char*>(pData);
     //cout << s << endl;
     delete[] pData;
 
-     // std::cout << "found: " <<d.getIdentifier() << std::endl;
-    m_vector->push_back(d.getIdentifier());
-        // the ID of this data entry is an answer to the query. I will just print it to stdout.
+    m_vector->push_back(static_cast<uint32_t>(d.getIdentifier()));


More information about the Liblas-commits mailing list