Skip to content

On the go refactoring

div72 edited this page Feb 21, 2022 · 6 revisions

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.

Unnecessary double conversions

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.

Redundant std::string initialization

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.

Long iterator constructs

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.