mirror of
https://github.com/XRPLF/rippled.git
synced 2025-11-20 02:55:50 +00:00
94 lines
2.8 KiB
Plaintext
94 lines
2.8 KiB
Plaintext
--------------------------------------------------------------------------------
|
|
TODO
|
|
--------------------------------------------------------------------------------
|
|
|
|
- Document in order:
|
|
SerializedType
|
|
STObject
|
|
SerializedLedgerEntry
|
|
|
|
- Replace uint160, uint256 in argument lists, template parameter lists, and
|
|
data members with tyepdefs from ripple_ProtocolTypes.h
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
LoadEvent
|
|
|
|
Is referenced with both a shared pointer and an auto pointer.
|
|
|
|
JobQueue
|
|
|
|
getLoadEvent and getLoadEventAP differ only in the style of pointer
|
|
container which is returned. Unnecessary complexity.
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
Naming
|
|
|
|
Some names don't make sense.
|
|
|
|
LedgerAcquire
|
|
Not a noun.
|
|
Is it really an InboundLedger ?
|
|
Does it continue to exist after the ledger is received?
|
|
|
|
Inconsistent names
|
|
|
|
We have full names like SerializedType and then acronyms like STObject
|
|
Two names for some things, e.g. SerializedLedgerEntry and SLE
|
|
|
|
Shared/Smart pointer typedefs in classes have a variety of different names
|
|
for the same thing. e.g. "pointer", "ptr", "ptr_t", "wptr"
|
|
|
|
Verbose names
|
|
|
|
The prefix "Flat" is more appealing than "Serialized" because its shorter and
|
|
easier to pronounce.
|
|
|
|
Ledger "Skip List"
|
|
|
|
Is not really a skip list data structure
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
Interfaces
|
|
|
|
Serializer
|
|
|
|
Upon analysis this class does two incompatible things. Flattening, and
|
|
unflattening. The interface should be reimplemented as two distinct
|
|
abstract classes, InputStream and OutputStream with suitable implementations
|
|
such as to and from a block of memory or dynamically allocated buffer.
|
|
|
|
The name and conflation of dual roles serves to confuse code at the point
|
|
of call. Does set(Serializer& s) flatten or unflatten the data? This
|
|
would be more clear:
|
|
bool write (OutputStream& stream);
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
Implementation
|
|
|
|
LoadManager
|
|
|
|
What is going on in the destructor
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
boost
|
|
|
|
Unclear from the class declaration what style of shared object management
|
|
is used. Prefer to derive from a ReferenceCountedObject class so that the
|
|
behavior is explicit. Furthermore the use of intrusive containers is
|
|
preferred over the alternative.
|
|
|
|
make_shared <> () is awkward.
|
|
|
|
boost::recursive_mutex
|
|
|
|
Recursive mutexes should never be necessary.
|
|
They require the "mutable" keyword for const members to acquire the lock (yuck)
|
|
|
|
Replace recursive_mutex with juce::Mutex to remove boost dependency
|
|
|