RPC client: Simplify command line string-to-JSON-value conversion code #4415

pull jgarzik wants to merge 1 commits into bitcoin:master from jgarzik:rpc-type-map changing 1 files +104 −79
  1. jgarzik commented at 11:38 pm on June 25, 2014: contributor

    By default, all command line parameters are converted into JSON string values. There is no need to manually specify the incoming type.

    A binary decision “parse as string or JSON?” is all that’s necessary.

    Convert to a simple class, initialized at runtime startup, which offers a quick lookup to answer “parse as JSON?” conversion question.

    Future parameter conversions need only to indicate the method name and zero-based index of the parameter needing JSON parsing.

  2. in src/rpcclient.cpp: in fbed7aa680 outdated
    15@@ -15,92 +16,117 @@
    16 using namespace std;
    17 using namespace json_spirit;
    18 
    19-template<typename T>
    20-void ConvertTo(Value& value, bool fAllowNull=false)
    21+class CRPCConvertParam
    22 {
    23-    if (fAllowNull && value.type() == null_type)
    


    laanwj commented at 5:34 am on June 26, 2014:
    This removes the fAllowNull check. Not that I think that’s critical, the server will validate the arguments.

    jgarzik commented at 12:19 pm on June 26, 2014:
    Yes, that’s intentional. It’s not needed and adds no value.
  3. in src/rpcclient.cpp: in fbed7aa680 outdated
    194+        // insert string value directly
    195+        if (!rpcCvtTable.convert(strMethod, idx))
    196+            params.push_back(strVal);
    197+
    198+        // parse string as JSON, insert bool/number/object/etc. value
    199+        else {
    


    laanwj commented at 5:43 am on June 26, 2014:
    Nit: Either don’t add an empty line before the else, or surround the first part with braces as well, it looks like there is no else clause and the if(...) ...; is separate while reading this code quickly.

    Diapolo commented at 12:12 pm on June 26, 2014:
    Agreed, this is pretty ugly that way…
  4. gavinandresen commented at 3:51 pm on June 26, 2014: contributor

    ACK because ‘better is better.’

    Nit: this could probably be even simpler using boost::assign::insert and assigning into a multimap<string method, int paramIdx>. See http://www.boost.org/doc/libs/1_55_0b1/libs/assign/doc/index.html

  5. laanwj commented at 4:04 pm on June 26, 2014: member
    Yes, ACK after code formatting nit.
  6. RPC client: Simplify command line string-to-JSON-value conversion code
    By default, all command line parameters are converted into JSON string
    values.  There is no need to manually specify the incoming type.
    
    A binary decision "parse as string or JSON?" is all that's necessary.
    
    Convert to a simple class, initialized at runtime startup, which offers
    a quick lookup to answer "parse as JSON?" conversion question.
    
    Future parameter conversions need only to indicate the method name
    and zero-based index of the parameter needing JSON parsing.
    e35b37b1b8
  7. jgarzik commented at 2:42 am on June 27, 2014: contributor
    Rebased + Updated to add braces.
  8. BitcoinPullTester commented at 2:53 am on June 27, 2014: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4415_e35b37b1b8252c3d3f66001ba1efccd263307add/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
  9. laanwj merged this on Jun 30, 2014
  10. laanwj closed this on Jun 30, 2014

  11. laanwj referenced this in commit 5c184cb850 on Jun 30, 2014
  12. in src/rpcclient.cpp: in e35b37b1b8
    195+
    196+        // insert string value directly
    197+        if (!rpcCvtTable.convert(strMethod, idx)) {
    198+            params.push_back(strVal);
    199+        }
    200+
    


    Diapolo commented at 2:24 pm on June 30, 2014:
    @jgarzik Let me ask this time :), is there still an open pull, which changes this to something more readable in the near future?

    jgarzik commented at 11:41 pm on July 13, 2014:
    @Diapolo Feel free to submit a cleanup PR if you desire! Thanks for asking.
  13. jgarzik deleted the branch on Aug 24, 2014
  14. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 21:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me