In commit “scripted-diff: rename block and undo functions for consistency” (2ff0ea366c61b2fcb80ad1f711480915c7a7aa2e)
Agree with hodlinator that Read/Write or Load/Save would now be better than Read/Save. The reason I suggested Save instead of Write in #31490#pullrequestreview-250956282 is that previously we had a high level function for writing called Save and lower level functions called Write, but now after 9b59f8b624cc641bd7216ececffa3111041fd760, the lower level functions are dropped, so there’s no need to have a naming distinction.
I liked the explicitness of ToDisk/FromDisk for cases where that is actually what it’s doing, makes code more self-documenting, even if slightly verbose.
One reason I think it is good to drop ToDisk/FromDisk suffixes is for consistency. Lots of block storage functions (particularly WriteUndoDataForBlock here) access the disk but don’t have “disk” in the name so having “disk” some places and not others without having a clear rule for where it is used seems likely to lead to misinterpretation and false inferences.
But the main reason I think it is good to drop “disk” from the names is that the main functions here (the high-level functions called by application code, not the low-level functions called by benchmarks) do not accept any parameters referencing files or disk locations:
0bool ReadBlock(CBlock& block, const CBlockIndex& index) const;
1FlatFilePos SaveBlock(const CBlock& block, int nHeight);
2bool ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const;
3bool SaveBlockUndo(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block);
These functions don’t need to use disk storage, even if that is what they are doing now. They could just as easily be using any type of storage (cloud, mobile, browser, etc).
However, I do think it would be good to call these Read/Write or Load/Save instead of Read/Save. And if we want to add ToDisk/FromDisk suffixes to lower level functions that seems ok as long as it is done consistently.
