Iterate over properties #191
Conversation
There was a problem hiding this comment.
Looks good. Just a few minor comments.
One thing to consider is the removal of properties such as Atom.remove("my prop");, but that is probably the subject of a different PR. Such code can now be tested because we can iterate over the properties themselves.
I'll need to update the SDF writer now because I want to save atom properties with this format.
include/chemfiles/Property.hpp
Outdated
| @@ -213,6 +215,20 @@ class property_map final { | |||
| /// Get the property with the given `name` if it exists. | |||
| optional<const Property&> get(const std::string& name) const; | |||
|
|
|||
| size_t size() const { | |||
| @@ -75,87 +75,87 @@ extern "C" chfl_status chfl_topology_remove(CHFL_TOPOLOGY* const topology, uint6 | |||
| ) | |||
| } | |||
|
|
|||
| extern "C" chfl_status chfl_topology_bonds_count(const CHFL_TOPOLOGY* const topology, uint64_t* nbonds) { | |||
| extern "C" chfl_status chfl_topology_bonds_count(const CHFL_TOPOLOGY* const topology, uint64_t* const count) { | |||
There was a problem hiding this comment.
I like the new consistency in variable names!
| const char* names[2] = {NULL}; | ||
| chfl_atom_list_properties(atom, names, count); | ||
|
|
||
| // Properties are not ordered |
There was a problem hiding this comment.
I'm guessing this is implementation dependent due to an unordered map.
There was a problem hiding this comment.
Yes. I think it it better to document it as not ordered for now, this can be changed later without breaking code
There was a problem hiding this comment.
This is fine, I just wanted to make sure.
Luthaf
force-pushed
the
iterate-properties
branch
from
f4fc6ca
to
f656945
Compare
Luthaf
force-pushed
the
iterate-properties
branch
from
f656945
to
907b05c
Compare
This replaces #163