Move repeated read / write logic to base Serializable class #709
Conversation
|
By analyzing the blame information on this pull request, we identified @scottpurdy, @subutai and @breznak to be potential reviewers |
|
@scottpurdy Requesting an early review, before I update all the other classes that implement serialization. Is this approach okay? |
| #include <nupic/math/SparseBinaryMatrix.hpp> | ||
| #include <nupic/math/SparseMatrix.hpp> | ||
| #include <nupic/proto/SpatialPoolerProto.capnp.h> | ||
| #include <nupic/types/Types.hpp> | ||
|
|
||
| using namespace std; | ||
| using namespace nupic::base::serializable; |
There was a problem hiding this comment.
It's generally bad practice to have using statements in headers since it brings the contents into scope for anything that includes this header.
|
done reviewing. The Serializable class looks great! |
|
@scottpurdy Thanks! I've incorporated your feedback. I'll update the remaining serializable classes now. |
| @@ -66,7 +67,7 @@ namespace nupic { | |||
| * } | |||
| * | |||
| */ | |||
| class SpatialPooler { | |||
| class SpatialPooler : public nupic::Serializable<SpatialPoolerProto> { | |||
There was a problem hiding this comment.
we are already inside the nupic namespace so you shouldn't need to qualify Serializable
|
@scottpurdy Ready for review. |
|
👍 |
Move repeated read / write logic to base Serializable class
Fixes #708.