rippled/TODO.txt

--------------------------------------------------------------------------------
BEAST TODO
--------------------------------------------------------------------------------

- Remove anything having to do with DLL builds like
  BEAST_DLL, BEAST_DLL_BUILD, BEAST_DISABLE_DLL_ALLOCATORS

- Fix FifoFreeStoreWithTLS reference counting bug

- Implement a reasonable substitute for boost's thread_local_storage

- Think about doing away with BEAST_CALLTYPE and BEAST_API

- Decide if headers should just include BeastConfig.h instead of making the
  host program do it.

- Rename malloc/calloc JUCE members that conflict with the debug CRT from MSVC

- Make beast::HashMap support assignment via operator[]

- Reformat every Doxygen comment
- Fix Doxygen metatags
- update Beast Doxyfile

- Rename include guards to boost style, e.g. BEAST_THROW_H_INCLUDED

- Decide if we should get rid of AtomicCounter, AtomicFlag, AtomicPointer, AtomicState

- Clean up CacheLine, StaticObject

- Clean up ConcurrentObject

- Rename ConcurrentState to SharedState or something?

- Figure out what to do with ReadWriteLock, and NamedPipe which uses it?

- Put BEAST_PUBLIC_FUNCTION in front of all loose functions

- restructure the repo sources to look like this:
  /Source/beast/beast_core/beast_core.h
  etc...
- Put the BeastConfig.h at the root of the repo.
- Make sure the template BeastConfig.h is included in the Doxyfile

- Implement robust key/value database with bulk write

--------------------------------------------------------------------------------
RIPPLE TODO
--------------------------------------------------------------------------------

- Restructure the ripple sources to have this directory structure:
  /Source/ripple/ripple_core/ripple_core.h
  etc...

- Turn on MSVC debug CRT heap checking with file and line reporting <crtdbg>

- Replace all NULL with nullptr

- Rewrite TxFormats to use beast containers and RAII, and not leak
- Add ICore interface
- Make TxFormats a member of ICore instead of a singleton.

- Rename LoadMonitor to LoadMeter, change LoadEvent to LoadMeter::ScopedSample

- Rename include guards to boost style, e.g. RIPPLE_LOG_H_INCLUDED

- Replace C11X with BEAST_COMPILER_SUPPORTS_MOVE_SEMANTICS

- Fix all leaks on exit (!)

    Say there's a leak, a ledger that can never be accessed is locked in some
        structure. If the organized teardown code frees that structure, the leak
        will not be reported.
    Yes, so you'll detect some small subset of leaks that way.
    You'll still have to be vigilant for the leaks that won't detect.
    The problem is ordering. There are lots of circular dependencies.
    The biggest problem is the order of destruction of global objects. (I think)
    Getting rid of global objects is a good solution to that.
    Vinnie Falco: Those I can resolve with my ReferenceCountedSingleton. And
                  yeah thats a good approach, one that I am doing slowly anyway
    Yeah, that's good for other reasons too, not just the unpredictability of
        creation order that can hide bugs.
    There may also just be some missing destructors.
    Some of it may be things being shut down in the wrong order. Like if you shut
        down the cache and then something that uses the cache, objects may get
        put in the cache after it was shut down.

- Remove "ENABLE_INSECURE" when the time is right.

- lift unique_ptr / auto_ptr into ripple namespace, or replace with ScopedPointer

- Make LevelDB and Ripple code work with both Unicode and non-Unicode Windows APIs

- Raise the warning level and fix everything

- Go searching through VFALCO notes and fix everything

- Deal with function-level statics used for SqliteDatabase (like in
  HSBESQLite::visitAll)

- Document in order:
    SerializedType
    STObject
    SerializedLedgerEntry

- Replace uint160, uint256 in argument lists, template parameter lists, and
  data members with tyepdefs from ripple_ProtocolTypes.h

- Consolidate SQLite database classes: DatabaseCon, Database, SqliteDatabase.

--------------------------------------------------------------------------------

LoadEvent

    Is referenced with both a shared pointer and an auto pointer.
    Should be named LoadMeter::ScopedSample

JobQueue

    getLoadEvent and getLoadEventAP differ only in the style of pointer
    container which is returned. Unnecessary complexity.

--------------------------------------------------------------------------------

Naming

Some names don't make sense.

  Index
    Stop using Index to refer to keys in tables. Replace with "Key" ?
    Index implies a small integer, or a data structure.
    
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. This is more appropriately
  called an "index" although that name is currently used to identify hashes
  used as keys.

Duplicate Code

  LedgerEntryFormat and TxFormat

--------------------------------------------------------------------------------

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);

--------------------------------------------------------------------------------

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 beast::Mutex to remove boost dependency