Util: Remove ArgsManager wrappers: #10119

1. 
   jtimon commented at 7:22 am on March 30, 2017: contributor

   • ParseParameters
   • ReadConfigFile
   • ForceSetArg
   • SoftSetArg

   Includes:

   • Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs #9494

   From the wrappers newly introduced with ArgsManager, these seem currently the easiest to remove.

2. fanquake added the label Refactoring on Mar 30, 2017
3. in src/util.cpp:417 in 11b82c4e32 outdated

   378+        mapMultiArgs[str].push_back(strValue);
   379     }
   380 }
   381 
   382-bool IsArgSet(const std::string& strArg)
   383+std::vector<std::string> ArgsManager::ArgsAt(const std::string& strArg)
   

   ---

   ---

   

   JeremyRubin commented at 5:50 pm on March 30, 2017:

   Any reason to not return a const ref?

   Also, ArgsAt is a bit of an odd name, perhaps ArgsFor or something (but maybe not worth the effort to redo this)

   ---

   

   jtimon commented at 8:07 pm on April 5, 2017:

   See #9494 (review) for some reason all your reviews are appearing in #10119 instead of #9494 . Can you make the comments there for other reviewers to get them? Remember after #9494 this PR (#10119) is a single commit (that could be squashed to #9494 or not)

   ---

   

   JeremyRubin commented at 8:26 pm on April 5, 2017:

   oh – I didn’t understand ‘includes’ to mean builds on the PR. gotcha
4. in src/util.cpp:474 in 11b82c4e32 outdated

   442-void ForceSetArg(const std::string& strArg, const std::string& strValue)
   443+void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strValue)
   444 {
   445     LOCK(cs_args);
   446     mapArgs[strArg] = strValue;
   447+    mapMultiArgs[strArg].push_back(strValue);
   

   ---

   ---

   

   JeremyRubin commented at 5:55 pm on March 30, 2017:

   Should this push_back or replace?

   ---

   

   jtimon commented at 1:20 pm on April 4, 2017:

   I believe push_back, but it’s a good point, I am not sure now.
5. in src/util.cpp:477 in 11b82c4e32 outdated

   445     LOCK(cs_args);
   446     mapArgs[strArg] = strValue;
   447+    mapMultiArgs[strArg].push_back(strValue);
   448 }
   449 
   450+bool IsArgSet(const std::string& strArg)
   

   ---

   ---

   

   JeremyRubin commented at 5:56 pm on March 30, 2017:

   Is the PR too big if you get rid of these?

   ---

   

   jtimon commented at 3:17 pm on April 4, 2017:

   The less disruptive to remove are removed in #10119 I’m happy to squash that in #9494 Others are more disruptive to remove, but I’m working on doing it for SoftSetBoolArg and IsArgSet in https://github.com/jtimon/bitcoin/compare/0.14-args-wrappers...jtimon:0.14-args-isargset-wrapper but I’m not sure everybody will like that way of doing it.
6. in src/util.h:177 in 11b82c4e32 outdated

   130@@ -133,6 +131,16 @@ inline bool IsSwitchChar(char c)
   131 #endif
   132 }
   133 
   134+class ArgsManager
   135+{
   136+protected:
   137+    CCriticalSection cs_args;
   138+    std::map<std::string, std::string> mapArgs;
   139+    std::map<std::string, std::vector<std::string> > mapMultiArgs;
   

   ---

   ---

   

   JeremyRubin commented at 5:57 pm on March 30, 2017:

   why not use multimap?

   ---

   

   jtimon commented at 1:30 pm on April 4, 2017:

   I was just using the same that was used before. We can move to multimap, what would be the gain?

   ---

   

   JeremyRubin commented at 2:10 pm on April 4, 2017:

   No real gain, just slightly simpler (because multi_map is intended to be used I think exactly in this type of case).

   ---

   

   jtimon commented at 8:09 pm on April 5, 2017:

   I thought it was more for when you wanted the elements on the vector to be ordered in certain way or something. Leaving the same structure seems easier to review, but once encapsulated it is easy to make further improvements to ArgsManager’s internal implementation.
7. 
   JeremyRubin commented at 6:14 pm on March 30, 2017: contributor

   utack
8. jtimon force-pushed on Apr 4, 2017
9. 
   jtimon commented at 3:17 pm on April 4, 2017: contributor

   Needed rebase.
10. jtimon force-pushed on Apr 5, 2017
11. Util: Create ArgsManager class 8623e251b6
12. Util: Set _mapMultiArgs in ArgsManager::SoftSetArg, ForceSetArg, SoftSetBoolArg b9e117b1fa
13. Util: s/mapMultiArgs.count(/argsGlobal.IsArgSet(/ 2aa8d32585
14. Util: Introduce ArgsManager::ArgsAt() ce8a5d6f1a
15. Util: Put mapMultiArgs inside ArgsManager 996d2ae6ac
16. Util: Remove ArgsManager wrappers:

    - ParseParameters
    - ReadConfigFile
    - ForceSetArg
    - SoftSetArg

    9d6c531320
17. jtimon force-pushed on Apr 6, 2017
18. 
    jtimon commented at 1:11 pm on May 5, 2017: contributor

    Incorportated in #9494
19. jtimon closed this on May 5, 2017

    ---

20. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon JeremyRubin

Labels
Refactoring