Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Use std::vector API for construction of test data #15099

Merged
merged 1 commit into from Jan 4, 2019

Conversation

Projects
None yet
5 participants
@domob1812
Copy link
Contributor

commented Jan 4, 2019

For constructing test scripts, use std::vector and, in particular, std::vector::insert to insert 20 zero bytes rather than listing the full array of bytes explicitly. This makes the code easier to read and makes it immediately obvious what the structure of the data is, without having to count the zeros to understand it.

Of course, that is a matter of taste - so if you disagree that the change makes the code easier to read, let me know.

This has been split out of #14752.

Use std::vector API for construction of test data.
For constructing test scripts, use std::vector and, in particular,
std::vector::insert to insert 20 zero bytes rather than listing the full
array of bytes explicitly.  This makes the code easier to read and makes
it immediately obvious what the structure of the data is, without having
to count the zeros to understand it.
@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK

I don't think this is a very pressing change but OTOH I think it's marginally easier to read, and harder to make mistakes with—e.g. with a span of 0, it's easy to accidentally insert one too many or too few.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK 6b25f29

@promag

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK 6b25f29. Does it make sense to add the following?

CScript(const std::vector<unsigned char>& v) : CScript(v.begin(), v.end()) {}

which would simplify this change (and maybe in other places).

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Jan 4, 2019

Merge bitcoin#15099: tests: Use std::vector API for construction of t…
…est data

6b25f29 Use std::vector API for construction of test data. (Daniel Kraft)

Pull request description:

  For constructing test scripts, use `std::vector` and, in particular, `std::vector::insert` to insert 20 zero bytes rather than listing the full array of bytes explicitly.  This makes the code easier to read and makes it immediately obvious what the structure of the data is, without having to count the zeros to understand it.

  Of course, that is a matter of taste - so if you disagree that the change makes the code easier to read, let me know.

  This has been split out of bitcoin#14752.

Tree-SHA512: af82d447f0077259049f1da2d6f86a6c29723c6e17bd342e9a9ecf37b13bddff40643af95c8b3a3260765a5591713d31ca8a45a5a0c20a12c139aee53ea150da

@MarcoFalke MarcoFalke merged commit 6b25f29 into bitcoin:master Jan 4, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@domob1812 domob1812 deleted the domob1812:p2sh-test-refactor branch Jan 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.