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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 ↗(On Diff #1156)

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

380 ↗(On Diff #1156)

dito

470 ↗(On Diff #1156)

dito

src/core_write.cpp
63 ↗(On Diff #1156)

Use C++ style constructor.

uint8_t(SIGHASH)

It'll be much more readable.

src/test/netbase_tests.cpp
279 ↗(On Diff #1156)

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 ↗(On Diff #1156)

C++ style constructor instead of C style casts.

src/test/scriptflags.cpp
17 ↗(On Diff #1156)

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 ↗(On Diff #1156)

Ok

src/core_write.cpp
63 ↗(On Diff #1156)

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

src/test/netbase_tests.cpp
279 ↗(On Diff #1156)

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

src/test/scriptflags.cpp
17 ↗(On Diff #1156)

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 updated this revision to Diff 1157.Aug 10 2017, 19:39
CCulianu edited edge metadata.

Better formatting (also implemented deadalnix's recommendations)

CCulianu added inline comments.Aug 10 2017, 19:42
src/test/netbase_tests.cpp
287 ↗(On Diff #1156)

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 ↗(On Diff #1156)

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

src/test/netbase_tests.cpp
279 ↗(On Diff #1156)

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.