IMO this is less ugly instead of more ugly than #1102. All non-implementation information about RPC commands is conveniently in one place, in table vRpcCommands. You could in principle add the help message as well. And there's no long registration function that has to be kept up to date when new commands added, just a simple one.

One remark though: can you move the RegisterRPCCommands() call to AppInit2() instead? This make sure that the RPC commands data structure is initialized even if the RPC thread is not started. #1075 makes use of this for executing local "RPC" commands without -server.

You could have an RPCDIspatcher class with a singleton object, which uses RegisterRPCCommands as constructor.

As I understand it we currently simply take both mutexes for all RPC commands? (which is inefficient, but very safe) Did that change with this commit?

Oh right - I confused safe mode with "requires extra locking". I removed the part of the comment about that.

BTW, I am looking for suggestions on how to best encapsulate the parameter parsing in CommandLineRPC(). That is the last area where there is a long "if command==foo, do X" decision table, containing per-RPC-command logic.

It would be nice to move that parameter parsing into the vRPCCommand table.

I'm not entirely sure about that. IMO it would be nice to separate client and server concerns as far as possible, even into separate implementation files.

• The server logic is a box that accepts JSON and produces JSON data structures. It knows nothing of argument parsing, formatting, just handles commands.
• The client logic takes a command name and list of (string) arguments, and produces a string message and error code as output. The conversion from argument list to JSON internally is dependent on the command. It can either communicate with the server through the network (as JSON client) or by passing the JSON object in and out in memory (UI console).

There's some network code and such that exists in limbo between the two.

(I have nothing against making the "convert argument strings to JSON" data-driven as well, but I'm not convinced it belongs in the same place)

RE @laanwj "And there's no long registration function that has to be kept up to date when new commands added, just a simple one"

Well, the long registration function is replaced with a long table. As the saying goes, "six of one, half-dozen of the other" In both this pull request and #1102, there is one (1) place that contains a master list of RPC commands. In this pull request it is vRPCCommands[], and in #1102 it is RegisterRPCCommands().

I do lean towards this pull request too, because it is smaller and more compact. However, the continued use of 'rpcfn_type' would probably be ugly to C++ purists :)

ACK. I like this one better.

I like data-driven programming, so describing argument types in a std::map<string method, std::map<int argNumber, ...something... argType> > or something instead of the current big long "if/else..." appeals to me. But that's such a low priority I probably wouldn't bother.

I've extended the encapsulation a bit further (while fixing the initialization issue) in #1124.

Included in #1124