Based on conversation in #13462, it seems the serialization GetType has very narrow use/effect. In every case except for CAddress, which specifically relates to a network peer's address, not a wallet address etc., the serialized representation of an object is irrespective of its destination / type.
This removes the unused GetType method from CSizeComputer as a step to further narrowing that use.
<!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
utACK 60cf69ee7855b8d5c86d3ed41c47f764ccee5095. Nice simplification.
I wonder if this is slightly confusing as long as long as it still uses the type parameter in the rest of (de)serialization.
Long term I do think that it makes sense to get rid of it altogether. Instead of passing a type into the serialization it'd be more clear to use a subclass or proxy object if different kinds of (de)serialization are wanted.
<!--5d09a71f8925f3f132321140b44b946d-->The last travis run for this pull request was 58 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
900 | @@ -901,10 +901,9 @@ class CSizeComputer 901 | protected: 902 | size_t nSize; 903 | 904 | - const int nType; 905 | const int nVersion; 906 | public: 907 | - CSizeComputer(int nTypeIn, int nVersionIn) : nSize(0), nType(nTypeIn), nVersion(nVersionIn) {} 908 | + CSizeComputer(int nVersionIn) : nSize(0), nVersion(nVersionIn) {}
Should be explicit? :-)
yes, that's good practice for one-argument constructors (sorry for fat fingering the resolve button)
Now that `GetType()` is not propagated, the benefits are not worth the code.
Made the now single-arg constructor explicit
Edit: Removed. Not really fair of me to ask such a thing. Still not sure how to fix the code but will work it out myself.