From 7a11d761e7608e807cef0b913915f15ffae499e9 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Wed, 3 Feb 2016 16:18:39 +0100
Subject: [PATCH] Refactoring: Implementation details in separate files.
Moved SWC parser's and cell record's implementation details to a
separate file. Also added braces around single-line control/loop blocks.
---
CMakeLists.txt | 6 +-
src/CMakeLists.txt | 8 ++
src/swcio.cpp | 214 +++++++++++++++++++++++++++++++++++++++++++
src/swcio.hpp | 181 ++----------------------------------
tests/CMakeLists.txt | 11 +--
tests/test_swcio.cpp | 7 +-
6 files changed, 241 insertions(+), 186 deletions(-)
create mode 100644 src/CMakeLists.txt
create mode 100644 src/swcio.cpp
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 717eb124..ea981fe4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,11 +11,11 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -Wall")
set(CMAKE_EXPORT_COMPILE_COMMANDS "YES")
# generated .a and .so go into /lib
-#set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
-#set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
include_directories(${CMAKE_SOURCE_DIR}/src)
include_directories(${CMAKE_SOURCE_DIR})
+add_subdirectory(src)
add_subdirectory(tests)
-
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
new file mode 100644
index 00000000..90c9d8f8
--- /dev/null
+++ b/src/CMakeLists.txt
@@ -0,0 +1,8 @@
+set(HEADERS
+ swcio.hpp
+ )
+set(BASE_SOURCES
+ swcio.cpp
+)
+
+add_library(cellalgo ${BASE_SOURCES} ${HEADERS})
diff --git a/src/swcio.cpp b/src/swcio.cpp
new file mode 100644
index 00000000..129f70af
--- /dev/null
+++ b/src/swcio.cpp
@@ -0,0 +1,214 @@
+#include <iomanip>
+#include <map>
+#include <sstream>
+#include <unordered_set>
+#include <swcio.hpp>
+
+namespace nestmc
+{
+namespace io
+{
+
+//
+// cell_record implementation
+//
+void cell_record::renumber(id_type new_id, std::map<id_type, id_type> &idmap)
+{
+ auto old_id = id_;
+ id_ = new_id;
+
+ // Obtain parent_id from the map
+ auto new_parent_id = idmap.find(parent_id_);
+ if (new_parent_id != idmap.end()) {
+ parent_id_ = new_parent_id->second;
+ }
+
+ check_consistency();
+ idmap.insert(std::make_pair(old_id, new_id));
+}
+
+void cell_record::check_consistency() const
+{
+ // Check cell type as well; enum's do not offer complete type safety,
+ // since you can cast anything that fits to its underlying type
+ if (type_ < 0 || type_ > custom) {
+ throw std::invalid_argument("unknown cell type");
+ }
+
+ if (id_ < 0) {
+ throw std::invalid_argument("negative ids not allowed");
+ }
+
+ if (parent_id_ < -1) {
+ throw std::invalid_argument("parent_id < -1 not allowed");
+ }
+
+ if (parent_id_ >= id_) {
+ throw std::invalid_argument("parent_id >= id is not allowed");
+ }
+
+ if (r_ < 0) {
+ throw std::invalid_argument("negative radii are not allowed");
+ }
+}
+
+std::istream &operator>>(std::istream &is, cell_record &cell)
+{
+ swc_parser parser;
+ parser.parse_record(is, cell);
+ return is;
+}
+
+
+std::ostream &operator<<(std::ostream &os, const cell_record &cell)
+{
+ // output in one-based indexing
+ os << cell.id_+1 << " "
+ << cell.type_ << " "
+ << std::setprecision(7) << cell.x_ << " "
+ << std::setprecision(7) << cell.y_ << " "
+ << std::setprecision(7) << cell.z_ << " "
+ << std::setprecision(7) << cell.r_ << " "
+ << ((cell.parent_id_ == -1) ? cell.parent_id_ : cell.parent_id_+1);
+
+ return os;
+}
+
+
+//
+// Utility functions
+//
+
+bool starts_with(const std::string &str, const std::string &prefix)
+{
+ return (str.find(prefix) == 0);
+}
+
+void check_parse_status(const std::istream &is)
+{
+ if (is.fail())
+ // If we try to read past the eof; fail bit will also be set
+ throw swc_parse_error("could not parse value");
+}
+
+template<typename T>
+T parse_value_strict(std::istream &is)
+{
+ T val;
+ check_parse_status(is >> val);
+
+ // everything's fine
+ return val;
+}
+
+// specialize parsing for cell types
+template<>
+cell_record::kind parse_value_strict(std::istream &is)
+{
+ cell_record::id_type val;
+ check_parse_status(is >> val);
+
+ // Let cell_record's constructor check for the type validity
+ return static_cast<cell_record::kind>(val);
+}
+
+//
+// swc_parser implementation
+//
+
+std::istream &swc_parser::parse_record(std::istream &is, cell_record &cell)
+{
+ while (!is.eof() && !is.bad()) {
+ // consume empty and comment lines first
+ std::getline(is, linebuff_);
+ if (!linebuff_.empty() && !starts_with(linebuff_, comment_prefix_))
+ break;
+ }
+
+ if (is.bad()) {
+ // let the caller check for such events
+ return is;
+ }
+
+ if (is.eof() &&
+ (linebuff_.empty() || starts_with(linebuff_, comment_prefix_))) {
+ // last line is either empty or a comment; don't parse anything
+ return is;
+ }
+
+ if (is.fail()) {
+ throw swc_parse_error("too long line detected");
+ }
+
+ std::istringstream line(linebuff_);
+ cell = parse_record(line);
+ return is;
+}
+
+cell_record swc_parser::parse_record(std::istringstream &is)
+{
+ auto id = parse_value_strict<int>(is);
+ auto type = parse_value_strict<cell_record::kind>(is);
+ auto x = parse_value_strict<float>(is);
+ auto y = parse_value_strict<float>(is);
+ auto z = parse_value_strict<float>(is);
+ auto r = parse_value_strict<float>(is);
+ auto parent_id = parse_value_strict<cell_record::id_type>(is);
+
+ // Convert to zero-based, leaving parent_id as-is if -1
+ if (parent_id != -1) {
+ parent_id--;
+ }
+
+ return cell_record(type, id-1, x, y, z, r, parent_id);
+}
+
+
+std::vector<cell_record> swc_read_cells(std::istream &is)
+{
+ std::vector<cell_record> cells;
+ std::unordered_set<cell_record::id_type> ids;
+
+ std::size_t num_trees = 0;
+ cell_record::id_type last_id = -1;
+ bool needsort = false;
+
+ cell_record curr_cell;
+ while ( !(is >> curr_cell).eof()) {
+ if (curr_cell.parent() == -1 && ++num_trees > 1) {
+ // only a single tree is allowed
+ break;
+ }
+
+ auto inserted = ids.insert(curr_cell.id());
+ if (inserted.second) {
+ // not a duplicate; insert cell
+ cells.push_back(curr_cell);
+ if (!needsort && curr_cell.id() < last_id) {
+ needsort = true;
+ }
+
+ last_id = curr_cell.id();
+ }
+ }
+
+ if (needsort) {
+ std::sort(cells.begin(), cells.end());
+ }
+
+ // Renumber cells if necessary
+ std::map<cell_record::id_type, cell_record::id_type> idmap;
+ cell_record::id_type next_id = 0;
+ for (auto &c : cells) {
+ if (c.id() != next_id) {
+ c.renumber(next_id, idmap);
+ }
+
+ ++next_id;
+ }
+
+ return std::move(cells);
+}
+
+} // end of nestmc::io
+} // end of nestmc
diff --git a/src/swcio.hpp b/src/swcio.hpp
index 63043904..ba52720a 100644
--- a/src/swcio.hpp
+++ b/src/swcio.hpp
@@ -2,10 +2,7 @@
#include <exception>
#include <iostream>
-#include <map>
-#include <sstream>
-#include <type_traits>
-#include <unordered_set>
+#include <string>
#include <vector>
namespace nestmc
@@ -15,11 +12,6 @@ namespace io
{
-static bool starts_with(const std::string &str, const std::string &prefix)
-{
- return (str.find(prefix) == 0);
-}
-
class cell_record
{
public:
@@ -147,41 +139,10 @@ public:
return 2*r_;
}
- void renumber(id_type new_id, std::map<id_type, id_type> &idmap)
- {
- auto old_id = id_;
- id_ = new_id;
-
- // Obtain parent_id from the map
- auto new_parent_id = idmap.find(parent_id_);
- if (new_parent_id != idmap.end()) {
- parent_id_ = new_parent_id->second;
- }
-
- check_consistency();
- idmap.insert(std::make_pair(old_id, new_id));
- }
+ void renumber(id_type new_id, std::map<id_type, id_type> &idmap);
private:
- void check_consistency() const
- {
- // Check cell type as well; enum's do not offer complete type safety,
- // since you can cast anything that fits to its underlying type
- if (type_ < 0 || type_ > custom)
- throw std::invalid_argument("unknown cell type");
-
- if (id_ < 0)
- throw std::invalid_argument("negative ids not allowed");
-
- if (parent_id_ < -1)
- throw std::invalid_argument("parent_id < -1 not allowed");
-
- if (parent_id_ >= id_)
- throw std::invalid_argument("parent_id >= id is not allowed");
-
- if (r_ < 0)
- throw std::invalid_argument("negative radii are not allowed");
- }
+ void check_consistency() const;
kind type_; // cell type
id_type id_; // cell id
@@ -216,50 +177,9 @@ public:
, comment_prefix_("#")
{ }
- std::istream &parse_record(std::istream &is, cell_record &cell)
- {
- while (!is.eof() && !is.bad()) {
- // consume empty and comment lines first
- std::getline(is, linebuff_);
- if (!linebuff_.empty() && !starts_with(linebuff_, comment_prefix_))
- break;
- }
-
- if (is.bad())
- // let the caller check for such events
- return is;
-
- if (is.eof() &&
- (linebuff_.empty() || starts_with(linebuff_, comment_prefix_)))
- // last line is either empty or a comment; don't parse anything
- return is;
-
- if (is.fail())
- throw swc_parse_error("too long line detected");
-
- std::istringstream line(linebuff_);
- cell = parse_record(line);
- return is;
- }
+ std::istream &parse_record(std::istream &is, cell_record &cell);
private:
- void check_parse_status(const std::istream &is)
- {
- if (is.fail())
- // If we try to read past the eof; fail bit will also be set
- throw swc_parse_error("could not parse value");
- }
-
- template<typename T>
- T parse_value_strict(std::istream &is)
- {
- T val;
- check_parse_status(is >> val);
-
- // everything's fine
- return val;
- }
-
// Read the record from a string stream; will be treated like a single line
cell_record parse_record(std::istringstream &is);
@@ -269,56 +189,7 @@ private:
};
-// specialize parsing for cell types
-template<>
-cell_record::kind swc_parser::parse_value_strict(std::istream &is)
-{
- cell_record::id_type val;
- check_parse_status(is >> val);
-
- // Let cell_record's constructor check for the type validity
- return static_cast<cell_record::kind>(val);
-}
-
-cell_record swc_parser::parse_record(std::istringstream &is)
-{
- auto id = parse_value_strict<int>(is);
- auto type = parse_value_strict<cell_record::kind>(is);
- auto x = parse_value_strict<float>(is);
- auto y = parse_value_strict<float>(is);
- auto z = parse_value_strict<float>(is);
- auto r = parse_value_strict<float>(is);
- auto parent_id = parse_value_strict<int>(is);
-
- // Convert to zero-based, leaving parent_id as-is if -1
- if (parent_id != -1)
- parent_id--;
-
- return cell_record(type, id-1, x, y, z, r, parent_id);
-}
-
-
-std::istream &operator>>(std::istream &is, cell_record &cell)
-{
- swc_parser parser;
- parser.parse_record(is, cell);
- return is;
-}
-
-
-std::ostream &operator<<(std::ostream &os, const cell_record &cell)
-{
- // output in one-based indexing
- os << cell.id_+1 << " "
- << cell.type_ << " "
- << std::setprecision(7) << cell.x_ << " "
- << std::setprecision(7) << cell.y_ << " "
- << std::setprecision(7) << cell.z_ << " "
- << std::setprecision(7) << cell.r_ << " "
- << ((cell.parent_id_ == -1) ? cell.parent_id_ : cell.parent_id_+1);
-
- return os;
-}
+std::istream &operator>>(std::istream &is, cell_record &cell);
//
// Reads cells from an input stream until an eof is encountered and returns a
@@ -327,47 +198,7 @@ std::ostream &operator<<(std::ostream &os, const cell_record &cell)
// For more information check here:
// https://github.com/eth-cscs/cell_algorithms/wiki/SWC-file-parsing
//
-std::vector<cell_record> swc_read_cells(std::istream &is)
-{
- std::vector<cell_record> cells;
- std::unordered_set<cell_record::id_type> ids;
-
- std::size_t num_trees = 0;
- cell_record::id_type last_id = -1;
- bool needsort = false;
-
- cell_record curr_cell;
- while ( !(is >> curr_cell).eof()) {
- if (curr_cell.parent() == -1 && ++num_trees > 1)
- // only a single tree is allowed
- break;
-
- auto inserted = ids.insert(curr_cell.id());
- if (inserted.second) {
- // not a duplicate; insert cell
- cells.push_back(curr_cell);
- if (!needsort && curr_cell.id() < last_id)
- needsort = true;
-
- last_id = curr_cell.id();
- }
- }
-
- if (needsort)
- std::sort(cells.begin(), cells.end());
-
- // Renumber cells if necessary
- std::map<cell_record::id_type, cell_record::id_type> idmap;
- cell_record::id_type next_id = 0;
- for (auto &c : cells) {
- if (c.id() != next_id)
- c.renumber(next_id, idmap);
-
- ++next_id;
- }
-
- return std::move(cells);
-}
+std::vector<cell_record> swc_read_cells(std::istream &is);
} // end of nestmc::io
} // end of nestmc
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index ba8b481a..7e787ba4 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -18,10 +18,9 @@ set(TEST_SOURCES
add_executable(test.exe ${TEST_SOURCES} ${HEADERS})
-#target_link_libraries(test_compiler LINK_PUBLIC compiler gtest)
-
-#set_target_properties( test.exe
-# PROPERTIES
-# RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/tests"
-#)
+target_link_libraries(test.exe LINK_PUBLIC cellalgo)
+set_target_properties(test.exe
+ PROPERTIES
+ RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/tests"
+)
diff --git a/tests/test_swcio.cpp b/tests/test_swcio.cpp
index ba62c90b..a3a7336a 100644
--- a/tests/test_swcio.cpp
+++ b/tests/test_swcio.cpp
@@ -117,14 +117,16 @@ TEST(swc_parser, invalid_input)
{
// Check non-parsable values
- std::istringstream is("1a 1 14.566132 34.873772 7.857000 0.717830 -1\n");
+ std::istringstream is(
+ "1a 1 14.566132 34.873772 7.857000 0.717830 -1\n");
cell_record cell;
EXPECT_THROW(is >> cell, swc_parse_error);
}
{
// Check invalid cell type
- std::istringstream is("1 10 14.566132 34.873772 7.857000 0.717830 -1\n");
+ std::istringstream is(
+ "1 10 14.566132 34.873772 7.857000 0.717830 -1\n");
cell_record cell;
EXPECT_THROW(is >> cell, std::invalid_argument);
}
@@ -214,6 +216,7 @@ TEST(swc_parser, from_allen_db)
while( !(fid >> node).eof()) {
nodes.push_back(std::move(node));
}
+
// verify that the correct number of nodes was read
EXPECT_EQ(nodes.size(), 1058u);
}
--
GitLab