-
Notifications
You must be signed in to change notification settings - Fork 172
On the go refactoring
As with all older code bases, Gridcoin could use some code polishing and adaptations to newer C++ standards. When doing the kind of refactorings where you replace one construct with another you have two options: you either change all the incorrect construct to the new one, or you change them while you're editing a function anyway. Changing all at once is nice for consistency but can produce large diffs and makes merging between branches more difficult. Changing only the code you are editing for another purpose anyway allows for a less radical transition to the new constructs.
This document is intended as a guide for things to change when doing other Gridcoin work.
In C++ the result of an operation is converted to double if either side is a double, so there is rarely a need to do any manual conversions.
Change
- dPriority += (double)nValueIn * nConf;
+ dPriority += nValueIn * nConf;
Additionally there are a lot of places where an integer is printed as a double.
Change
- printf("Tx Size for %s %f",tx.GetHash().GetHex().c_str(),(double)nTxSize);
+ printf("Tx Size for %s %i", tx.GetHash().GetHex().c_str(), nTxSize);
Rationale: It increases readability.
std::string is by default initialized to an empty string.
Change
- std::string strLabel = "";
+ std::string strLabel;
It also has a clear function.
Change
- miningcpid.enccpid = "";
+ miningcpid.enccpid.clear();
Rationale: Slightly increased readability.
Before range-based for loops in C++11 developers had to resort to BOOST_FOREACH to iterate over something iterable without having to construct their iterator pairs and extract the value. This is now in the C++ standard.
Change
- BOOST_FOREACH(unsigned char c, str)
+ for(unsigned char c : str)
Rationale: Increased readability and decreased build times due to removal of an expensive boost header.
Note: In order to do reverse iteration a reversion adapter has to be used.
C++11 introduced the auto keyword which automatically deduces a variable type so long constructs can be avoided.
Change
- for(map<string,StructCPID>::iterator ii=mvCPIDs.begin(); ii!=mvCPIDs.end(); ++ii)
+ for(auto ii=mvCPIDs.begin(); ii!=mvCPIDs.end(); ++ii)
// Or better
for(const auto& cpid : mvCPIDs)
Rationale: Greatly improved readability.