[refactor] encapsulate a unit's denomination in a class
ClosedPublic
Actions

Authored by majcosta on Jun 10 2021, 01:12.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC23dc845e6077: [refactor] encapsulate a unit's denomination in a class

Summary

a class that represents a coin's denomination (unit, subunit and decimals of precision) rather than two constants and a magic number that's scattered around a few functions

does not change behavior

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

allow_parameterizing_unit

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15957
Build 31814: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 31813: arc lint + arc unit

Event Timeline

majcosta created this revision.Jun 10 2021, 01:12

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 10 2021, 01:12

majcosta requested review of this revision.Jun 10 2021, 01:12

Harbormaster completed remote builds in B15952: Diff 28829.Jun 10 2021, 01:26

ValueFromAmount function still had the decimal places magical number code

Harbormaster completed remote builds in B15953: Diff 28830.Jun 10 2021, 03:36

You need to add some unit tests for this new Currency object

src/amount.cpp
34 ↗	(On Diff #28830)	TIL %*d
src/amount.h
173 ↗	(On Diff #28830)	A few things here: I had to read the comments to figure out how a unit could be an Amount, the names are probably not the best Do you plan to extend this ? Otherwise you can just keep it simple and turn it into a POD Why do you need decimals if you already have the other params ? I suppose you added the subunit thing with fractional satoshis in mind, but please take care to not create confusion by solving a problem that does not exist (yet)