Page MenuHomePhabricator

Removed all usages of boost::assign::list_of in favor of C++11 initializer lists
ClosedPublic

Authored by CCulianu on Aug 10 2017, 17:07.

Details

Summary

C++11 initializer lists are more elegant and easier to maintain and read. So I removed all usages of boost::assign::list_of in favor of them. Note that some of these files have never been linted before so they may have extra lint formatting changes too.

Test Plan

make check, rpc-tests.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
initlists
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 723
Build 723: arc lint + arc unit

Event Timeline

CCulianu created this revision.Aug 10 2017, 17:07
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 10 2017, 17:07
deadalnix requested changes to this revision.Aug 10 2017, 18:23

A few changes, but that looks nice already.

src/chainparams.cpp
208

Try to put these back in one string and see what clang format does.

380

dito

470

dito

src/core_write.cpp
63

Use C++ style constructor.

uint8_t(SIGHASH)

It'll be much more readable.

src/test/netbase_tests.cpp
279

I think you can do

std::vector<uint8_t>{0}

But I haven't followed the latest C++ crazyness. Also, put the comment on the line before the code. It'll get better formatting it also make blame clearer.

287

C++ style constructor instead of C style casts.

src/test/scriptflags.cpp
17

Is the std::string really necessary ?

This revision now requires changes to proceed.Aug 10 2017, 18:23
CCulianu planned changes to this revision.Aug 10 2017, 18:57

Thanks for the feedback. I just did a quick pass to change it over -- but your readability feedback is good. I'll change it and update the revision.

src/chainparams.cpp
208

Ok

src/core_write.cpp
63

Yeah, that will look better. Good point. Will-do.

src/test/netbase_tests.cpp
279

Yeah you can and it will be much more readable, good point.

src/test/scriptflags.cpp
17

No, it's not. Maybe it was for boos but for this initializer list it isn't. I'll change it -- it will be more readable. I don't think the casts are necessary either.

CCulianu edited edge metadata.Aug 10 2017, 19:39
CCulianu updated this revision to Diff 1157.

Better formatting (also implemented deadalnix's recommendations)

CCulianu added inline comments.Aug 10 2017, 19:42
src/test/netbase_tests.cpp
287

What do you think of this? Note due to the BOOST_CHECK macro, I had to do:

Vec8({1,2,3,4})

rather than the more succinct:

Vec8{1,2,3,4}
freetrader accepted this revision.Aug 10 2017, 21:12

All good from my side.

deadalnix accepted this revision.Aug 10 2017, 21:30
deadalnix added inline comments.
src/chainparams.cpp
205 ↗(On Diff #1157)

Looks CCheckpointData would deserve a constructor of it's own. Having objects being just bags of data that anyone can manipulate is not good. But maybe that's out of the scope here.

src/core_write.cpp
63

Nice, it doesn't looks like it is necessary at all.

src/test/netbase_tests.cpp
279

It works.

This revision is now accepted and ready to land.Aug 10 2017, 21:30
This revision was automatically updated to reflect the committed changes.