Having done some work in and around Serialisable lately, I feel that there are some things that could be improved w.r.t. the clarity of function names etc. This could either be through renaming the functions to something more obviously related to serialisation - e.g. serialiseFromMap instead of fromMap - or perhaps moving out all the helper functions that take and operate on a SerialisedValue into a separate namespace. That would leave Serialisable to provide the basic virtuals and TOML hooks. I appreciate that in many classes we have the functions are explicitly prefixed with Serialisable:: to demark them, but CLion is always keen to point out that this is not necessary, and it's correct!
Also, I consistently find toMap and fromMap confusing and, depending on which way the wind is blowing, can convince myself freely that "to" is both serialising "to" the node, or deserialising "to" a local object!
For optional values it might be nice to be able to write something like: serialiseIf(node, "tag", myQuantity) where myQuantity is a std::optional.
Final thought, we do a lot of assignments where the type needs to be specified (e.g. nBinned_ = toml::find<long>(node, "nBinned"); but could have a function to take the target data object and assume the conversion type given to toml::find is the same.
Having done some work in and around
Serialisablelately, I feel that there are some things that could be improved w.r.t. the clarity of function names etc. This could either be through renaming the functions to something more obviously related to serialisation - e.g.serialiseFromMapinstead offromMap- or perhaps moving out all the helper functions that take and operate on aSerialisedValueinto a separate namespace. That would leaveSerialisableto provide the basicvirtuals and TOML hooks. I appreciate that in many classes we have the functions are explicitly prefixed withSerialisable::to demark them, but CLion is always keen to point out that this is not necessary, and it's correct!Also, I consistently find
toMapandfromMapconfusing and, depending on which way the wind is blowing, can convince myself freely that "to" is both serialising "to" the node, or deserialising "to" a local object!For optional values it might be nice to be able to write something like:
serialiseIf(node, "tag", myQuantity)wheremyQuantityis astd::optional.Final thought, we do a lot of assignments where the type needs to be specified (e.g.
nBinned_ = toml::find<long>(node, "nBinned");but could have a function to take the target data object and assume the conversion type given totoml::findis the same.