don't use exceptions when non-exceptional methods are available (#389)

* don't use exceptions when non-exceptional methods are available

* fix catch in section_base.h
* fix catch in errorMessages.h
* write more tests so we increase the test coverage

include/morphio/errorMessages.h

@@ -48,11 +48,11 @@ struct DebugInfo {
 
     /** Get section's line number within morphology file */
     int32_t getLineNumber(uint32_t sectionId) const {
-        try {
-            return _lineNumbers.at(sectionId);
-        } catch (const std::out_of_range&) {
+        const auto it = _lineNumbers.find(sectionId);
+        if (it == _lineNumbers.end()) {
             return -1;
         }
+        return it->second;
     }
     /** Morphology filename */
     std::string _filename;

include/morphio/morphology.h

@@ -43,7 +43,7 @@ class Morphology
     /** Constructor from an already parsed file */
     explicit Morphology(const HighFive::Group& group, unsigned int options = NO_MODIFIER);
     /** Constructor from an instance of morphio::mut::Morphology */
-    explicit Morphology(mut::Morphology);
+    explicit Morphology(const mut::Morphology&);
 
     /** Return the soma object */
     Soma soma() const;
@@ -52,7 +52,7 @@ class Morphology
     Mitochondria mitochondria() const;
 
     /** Return the endoplasmic reticulum object */
-    const EndoplasmicReticulum endoplasmicReticulum() const;
+    EndoplasmicReticulum endoplasmicReticulum() const;
 
     /** Return the annotation object */
     const std::vector<Property::Annotation>& annotations() const;

include/morphio/mut/mitochondria.h

@@ -77,7 +77,7 @@ class Mitochondria
     /**
        Append a new root MitoSection
     **/
-    MitoSectionP appendRootSection(const Property::MitochondriaPointLevel& points);
+    MitoSectionP appendRootSection(const Property::MitochondriaPointLevel& pointProperties);
 
     /**
        Append a root MitoSection

include/morphio/section_base.h

@@ -124,19 +124,25 @@ T SectionBase<T>::parent() const {
 
 template <typename T>
 std::vector<T> SectionBase<T>::children() const {
+    const auto& section_children = properties_->children<typename T::SectionId>();
+
+    if (section_children.empty()) {
+        return {};
+    }
+
+    const auto it = section_children.find(static_cast<int>(id_));
+    if (it == section_children.end()) {
+        return {};
+    }
+
     std::vector<T> result;
-    try {
-        const std::vector<uint32_t>& _children = properties_->children<typename T::SectionId>().at(
-            static_cast<int>(id_));
-        result.reserve(_children.size());
-        for (uint32_t id : _children) {
-            result.push_back(T(id, properties_));
-        }
-
-        return result;
-    } catch (const std::out_of_range&) {
-        return result;
+    const std::vector<uint32_t> children = it->second;
+    result.reserve(children.size());
+    for (uint32_t id : children) {
+        result.push_back(T(id, properties_));
     }
+
+    return result;
 }
 
 

src/morphology.cpp

@@ -13,10 +13,73 @@
 #include "readers/morphologyHDF5.h"
 #include "readers/morphologySWC.h"
 
+namespace {
+
+void buildChildren(const std::shared_ptr<morphio::Property::Properties> properties) {
+    {
+        const auto& sections = properties->get<morphio::Property::Section>();
+        auto& children = properties->_sectionLevel._children;
+
+        for (unsigned int i = 0; i < sections.size(); ++i) {
+            const int32_t parent = sections[i][1];
+            children[parent].push_back(i);
+        }
+    }
+
+    {
+        const auto& sections = properties->get<morphio::Property::MitoSection>();
+        auto& children = properties->_mitochondriaSectionLevel._children;
+
+        for (unsigned int i = 0; i < sections.size(); ++i) {
+            const int32_t parent = sections[i][1];
+            children[parent].push_back(i);
+        }
+    }
+}
+
+morphio::SomaType getSomaType(long unsigned int num_soma_points) {
+    switch (num_soma_points) {
+    case 0:
+        return morphio::SOMA_UNDEFINED;
+    case 1:
+        return morphio::SOMA_SINGLE_POINT;
+    case 2:
+        return morphio::SOMA_UNDEFINED;
+    default:
+        break;
+    }
+    return morphio::SOMA_SIMPLE_CONTOUR;
+}
+
+morphio::Property::Properties loadURI(const std::string& source, unsigned int options) {
+    const size_t pos = source.find_last_of('.');
+    if (pos == std::string::npos) {
+        throw(morphio::UnknownFileType("File has no extension"));
+    }
+
+    // Cross-platform check of file existance
+    std::ifstream file(source.c_str());
+    if (!file) {
+        throw(morphio::RawDataError("File: " + source + " does not exist."));
+    }
+
+    std::string extension = source.substr(pos);
+
+    if (extension == ".h5" || extension == ".H5") {
+        return morphio::readers::h5::load(source);
+    } else if (extension == ".asc" || extension == ".ASC") {
+        return morphio::readers::asc::load(source, options);
+    } else if (extension == ".swc" || extension == ".SWC") {
+        return morphio::readers::swc::load(source, options);
+    }
+
+    throw(morphio::UnknownFileType("Unhandled file type: only SWC, ASC and H5 are supported"));
+}
+
+
+}  // namespace
+
 namespace morphio {
-void buildChildren(std::shared_ptr<Property::Properties> properties);
-SomaType getSomaType(long unsigned int num_soma_point);
-Property::Properties loadURI(const std::string& source, unsigned int options);
 
 Morphology::Morphology(const Property::Properties& properties, unsigned int options)
     : properties_(std::make_shared<Property::Properties>(properties)) {
@@ -42,7 +105,7 @@ Morphology::Morphology(const HighFive::Group& group, unsigned int options)
 Morphology::Morphology(const std::string& source, unsigned int options)
     : Morphology(loadURI(source, options), options) {}
 
-Morphology::Morphology(mut::Morphology morphology) {
+Morphology::Morphology(const mut::Morphology& morphology) {
     properties_ = std::make_shared<Property::Properties>(morphology.buildReadOnly());
     buildChildren(properties_);
 }
@@ -55,7 +118,7 @@ Mitochondria Morphology::mitochondria() const {
     return Mitochondria(properties_);
 }
 
-const EndoplasmicReticulum Morphology::endoplasmicReticulum() const {
+EndoplasmicReticulum Morphology::endoplasmicReticulum() const {
     return EndoplasmicReticulum(properties_);
 }
 
@@ -72,19 +135,21 @@ Section Morphology::section(uint32_t id) const {
 }
 
 std::vector<Section> Morphology::rootSections() const {
+    const auto& sections = properties_->children<morphio::Property::Section>();
+
+    if (sections.empty()) {
+        return {};
+    }
+
     std::vector<Section> result;
-    try {
-        const std::vector<uint32_t>& children =
-            properties_->children<morphio::Property::Section>().at(-1);
-        result.reserve(children.size());
-        for (auto id : children) {
-            result.push_back(section(id));
-        }
 
-        return result;
-    } catch (const std::out_of_range&) {
-        return result;
+    const std::vector<uint32_t>& children = sections.at(-1);
+    result.reserve(children.size());
+    for (auto id : children) {
+        result.push_back(section(id));
     }
+
+    return result;
 }
 
 std::vector<Section> Morphology::sections() const {
@@ -163,67 +228,4 @@ breadth_iterator Morphology::breadth_end() const {
     return breadth_iterator();
 }
 
-SomaType getSomaType(long unsigned int num_soma_points) {
-    try {
-        return std::map<long unsigned int, SomaType>{{0, SOMA_UNDEFINED},
-                                                     {1, SOMA_SINGLE_POINT},
-                                                     {2, SOMA_UNDEFINED}}
-            .at(num_soma_points);
-    } catch (const std::out_of_range&) {
-        return SOMA_SIMPLE_CONTOUR;
-    }
-}
-
-void buildChildren(std::shared_ptr<Property::Properties> properties) {
-    {
-        const auto& sections = properties->get<Property::Section>();
-        auto& children = properties->_sectionLevel._children;
-
-        for (unsigned int i = 0; i < sections.size(); ++i) {
-            const int32_t parent = sections[i][1];
-            children[parent].push_back(i);
-        }
-    }
-
-    {
-        const auto& sections = properties->get<Property::MitoSection>();
-        auto& children = properties->_mitochondriaSectionLevel._children;
-
-        for (unsigned int i = 0; i < sections.size(); ++i) {
-            const int32_t parent = sections[i][1];
-            children[parent].push_back(i);
-        }
-    }
-}
-
-Property::Properties loadURI(const std::string& source, unsigned int options) {
-    const size_t pos = source.find_last_of(".");
-    if (pos == std::string::npos) {
-        throw(UnknownFileType("File has no extension"));
-    }
-
-    // Cross-platform check of file existance
-    std::ifstream file(source.c_str());
-    if (!file) {
-        throw(RawDataError("File: " + source + " does not exist."));
-    }
-
-    std::string extension = source.substr(pos);
-
-    auto loader = [&source, &options, &extension]() {
-        if (extension == ".h5" || extension == ".H5") {
-            return readers::h5::load(source);
-        }
-        if (extension == ".asc" || extension == ".ASC") {
-            return readers::asc::load(source, options);
-        }
-        if (extension == ".swc" || extension == ".SWC") {
-            return readers::swc::load(source, options);
-        }
-        throw(UnknownFileType("Unhandled file type: only SWC, ASC and H5 are supported"));
-    };
-
-    return loader();
-}
-
 }  // namespace morphio

src/mut/mito_section.cpp

@@ -82,12 +82,7 @@ std::shared_ptr<MitoSection> MitoSection::parent() const {
 }
 
 bool MitoSection::isRoot() const {
-    try {
-        parent();
-        return false;
-    } catch (const std::out_of_range&) {
-        return true;
-    }
+    return mitochondria_->parent_.count(id()) == 0;
 }
 
 bool MitoSection::hasSameShape(const MitoSection& other) const noexcept {

src/mut/mitochondria.cpp

@@ -10,7 +10,7 @@ namespace mut {
 
 Mitochondria::MitoSectionP Mitochondria::appendRootSection(const morphio::MitoSection& section_,
                                                            bool recursive) {
-    const auto ptr = std::make_shared<MitoSection>(this, _counter, section_);
+    auto ptr = std::make_shared<MitoSection>(this, _counter, section_);
     _register(ptr);
     root_sections_.push_back(ptr);
 
@@ -25,7 +25,7 @@ Mitochondria::MitoSectionP Mitochondria::appendRootSection(const morphio::MitoSe
 
 Mitochondria::MitoSectionP Mitochondria::appendRootSection(const MitoSectionP& section_,
                                                            bool recursive) {
-    const auto section_copy = std::make_shared<MitoSection>(this, _counter, *section_);
+    auto section_copy = std::make_shared<MitoSection>(this, _counter, *section_);
     _register(section_copy);
     root_sections_.push_back(section_copy);
 
@@ -40,7 +40,7 @@ Mitochondria::MitoSectionP Mitochondria::appendRootSection(const MitoSectionP& s
 
 Mitochondria::MitoSectionP Mitochondria::appendRootSection(
     const Property::MitochondriaPointLevel& pointProperties) {
-    const auto ptr = std::make_shared<MitoSection>(this, _counter, pointProperties);
+    auto ptr = std::make_shared<MitoSection>(this, _counter, pointProperties);
     _register(ptr);
     root_sections_.push_back(ptr);
 
@@ -70,12 +70,7 @@ const Mitochondria::MitoSectionP& Mitochondria::parent(const MitoSectionP& paren
 }
 
 bool Mitochondria::isRoot(const MitoSectionP& section_) const {
-    try {
-        parent(section_);
-        return false;
-    } catch (const std::out_of_range&) {
-        return true;
-    }
+    return parent_.count(section_->id()) == 0;
 }
 
 const Mitochondria::MitoSectionP& Mitochondria::section(uint32_t id) const {
@@ -86,14 +81,13 @@ void Mitochondria::_buildMitochondria(Property::Properties& properties) const {
     int32_t counter = 0;
     std::map<uint32_t, int32_t> newIds;
 
-    for (std::shared_ptr<MitoSection> mitoStart : root_sections_) {
+    for (const std::shared_ptr<MitoSection>& mitoStart : root_sections_) {
         std::queue<std::shared_ptr<MitoSection>> q;
         q.push(mitoStart);
         while (!q.empty()) {
             std::shared_ptr<MitoSection> section_ = q.front();
             q.pop();
-            bool root = isRoot(section_);
-            int32_t parentOnDisk = root ? -1 : newIds[parent(section_)->id()];
+            int32_t parentOnDisk = isRoot(section_) ? -1 : newIds[parent(section_)->id()];
 
             properties._mitochondriaSectionLevel._sections.push_back(
                 {static_cast<int>(properties._mitochondriaPointLevel._diameters.size()),
@@ -102,7 +96,7 @@ void Mitochondria::_buildMitochondria(Property::Properties& properties) const {
 
             newIds[section_->id()] = counter++;
 
-            for (auto child : children(section_)) {
+            for (const auto& child : children(section_)) {
                 q.push(child);
             }
         }
@@ -138,7 +132,7 @@ mito_upstream_iterator Mitochondria::upstream_end() const {
 }
 
 uint32_t Mitochondria::_register(const MitoSectionP& section) {
-    if (sections_.count(section->id())) {
+    if (sections_.count(section->id()) > 0) {
         throw SectionBuilderError("Section already exists");
     }
     _counter = std::max(_counter, section->id()) + 1;

tests/CMakeLists.txt

@@ -1,7 +1,8 @@
 set(TESTS_SRC
         main.cpp
-        test_morphology_readers.cpp
         test_immutable_morphology.cpp
+        test_mitochondria.cpp
+        test_morphology_readers.cpp
         test_mutable_morphology.cpp
         test_vasculature_morphology.cpp
         )

tests/test_3_h5.py

@@ -68,7 +68,7 @@ def test_soma_no_neurite():
                         [ 1.,  0.,  0.],
                         [ 0., 25.,  0.]])
     assert_array_equal(n.soma.diameters,
-                       [6,6,15])
+                       [6, 6, 15])
 
     assert len(n.root_sections) == 0