Remove unecessary friend keyword from the class definition #10194

pull bulldozer00 wants to merge 3 commits into bitcoin:master from bulldozer00:patch-1 changing 1 files +9 −8

bulldozer00 commented at 10:27 AM on April 12, 2017: none

The fewer the friends in a C++ code base, the better. Increased readability, less invasive coupling, better style. Also, if used, friend funcs should only declared within the scope of a class, and then defined outside of the class.

Remove unecessary friend keyword from the class definition

The fewer the friends in a C++ code base, the better. Increased readability, less coupling, better style. Also, if used, friend funcs should only declared within the scope of a class, and then defined outside of the class.

f97a2cf969

laanwj commented at 10:54 AM on April 12, 2017: member

This breaks the build:

In file included from txmempool.h:17:0,
                 from blockencodings.cpp:12:
coins.h:137:53: error: ‘bool CCoins::operator==(const CCoins&, const CCoins&)’ must take exactly one argument
     bool operator==(const CCoins &a, const CCoins &b) {
                                                     ^
coins.h:147:53: error: ‘bool CCoins::operator!=(const CCoins&, const CCoins&)’ must take exactly one argument
     bool operator!=(const CCoins &a, const CCoins &b) {
                                                     ^

laanwj added the label Refactoring on Apr 12, 2017
Fix bug introduced in last commit
binary operator==() only takes one arg
50555b7c13
Fix bug introduced in prev commit
Oops! :)
63dee6a0ca
bulldozer00 commented at 11:02 AM on April 12, 2017: none

My bad. Put the change in too quick. Should be good to go now?
MarcoFalke commented at 12:20 PM on April 12, 2017: member

Please try a make check on your machine.

On Wed, Apr 12, 2017 at 1:02 PM, bulldozer00 notifications@github.com wrote:

My bad. Put the change in too quick. Should be good to go now?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/10194#issuecomment-293544077, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv17ZxtB9xpJNXHtMdsWDIzatMELxks5rvK9agaJpZM4M7PNP .
bulldozer0 commented at 1:47 PM on April 12, 2017: none

Will do. Won't be able to try/fix locally until /later on today.
bulldozer00 commented at 6:40 PM on April 12, 2017: none

Ok guys, the unit test that I broke was a good one. By attempting to compare a const object with a non-const one for euality, it showed me that the operator==() and operator!=() functions must be declared as "friends" of the class. Rather than be pedantic and changing the code trivially so that the friend declarations remain within the class but the definitions be pulled outside of it, I'm going to close out this pull request.

Sorry for the inconvenience. It's my first try at learning Git, the GitHub workflow, and the Bitcoin build system. I promise to be more careful in the future.
bulldozer00 closed this on Apr 12, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels