I used to carefully design my C++ code with public / private / protected / friend (and the pointer-to-impl pattern for "compile time private") but a few years ago I started doing everything all-public ala python (struct instead of class by default) with occasionally an m_ prefix for "intended to be private", and I've never looked back. It's been great, made my programming life much easier.
This badge thing is clever, but seems like yet more accidental/unnecessary complexity on top of the already-probably-unnecessary private/protected/friend - as requirements change you now have this extra layer of access-control cruft to reason about, that you probably wouldn't even miss if it didn't exist.
Not to get too ranty, and no disrespect to Andreas, but the C++ culture seems to thrive in unnecessary / accidental complexity, deep template hacks and the like that add friction to refactoring, obfuscate underlying logic so you miss opportunities to simplify, and turn straightforward programming problems into brain-bending template puzzles.
OOP also; I've seen so many crazy class architectures that could just be a handful of plain-old-functions, complex virutal "type families" that could just be a TypeOfThingThisIs enum with some if statements in the respective functions, etc.
I don't know the Serenity codebase and use-case very well, and perhaps this kind of thing has a place in large libraries, but honestly even then I'd lean towards just making everything simple & public and use a prefix when needed, or describe appropriate usage in documentation. Seems to work in the python world (minus a few outlier codebases that go OOP-crazy).
Everything public versions poorly when you can't update all clients of your code after you change your implementation. It increases coupling in ways you can't control.
It also doesn't work very well with code completion - implementation methods get mixed up with surface API - and usability by third parties suffers.
If you're on a team of one, and you own the project for its whole lifetime, then you're fine.
Admittedly my use case is generally projects that are under my control.
But in the case of say an open source library, how likely are clients going to update their code with each new release of the library? Typical case is to do it from time to time, especially with compiled code / shared libs, and expect a few incompatibilities to sort out and fix when you do.
In a fast evolving large team project, seems it depends on how well the boundaries of the subsystems are worked out. Misusing supposed-to-be-private members is one minor possibility amidst many challenges.
In larger codebases with many developers, attention to the API is required in order to be able to maintain code without breaking clients.
The main downside of Badge<T> is that it comes with a performance penalty and ABI change for something that should be statically deducible (empty struct is one byte in C++).
The example also shows a call to a different translation unit. There's no code for passing the empty struct. There's also none for handling it, as you can see from the assembly output.
The one byte rule does nothing here, because its value is undefined and its address not observable.
edit: if you find a case where an empty struct parameter causes code to be emitted, let's consider filing bugs.
Oops, I did miss that, but this seems to be compiler or calling convention specific. x86_64 gcc 9.1, linked, shows no difference. However arm64 gcc 8.2 and x86-64 clang show a difference.
Don't you have to have pretty high performance needs for this class to make a performance difference? Unless it's called multiple thousand times per second I don't see it making much of a difference.
You can make register_device() an inline function that just calls a do_register_device() private function without the badge. That way you don't have to pay for the empty class.
I'm not sure I follow... The motivation for Badge is to have methods on VFS that only Device can call, without exposing VFS's internals to Device. Without Badge, what prevents non-Device functions from calling register_device()?
I guess the inline function would require a Badge and the private member function would not. So an inline constructed but never used Badge is even more likely to be completely optimized away
> OOP also; I've seen so many crazy class architectures that could just be a handful of plain-old-functions,
also, tons of "generic" numerical code with templates everywhere, where the only two possible instantiations are "float" and "double". Some people do really like to add useless complexity everywhere.
Eh... I have seen the same in Rust. People write tests with i32. i32 implements the Copy trait which means your can blindly clone it all over the place. When you try using some libraries with String or anything else they don't work.
Not only is it needlessly complex, but untested generic code is a minefield of subtle future bugs ... "oh, the function is there and tested, so it should work" ... well, not in this particular combination of the completely unrestricted generic parameter set.
I don't understand why you have been downvoted for asking "why m_?". Is everybody on here supposed to know everything about everything, else you get downvoted into grey unreadability?
Personally, I think that bashing/penalizing ignorance (and I mean the word ignorance literally, as being uninformed about a topic, no offense here whatsoever) is one of the lowest forms of abuse. It says much more about the downvoter than it does about the downvoted. And this not even considering the fact that searching online for "m_" isn't going to help you answer your own question.
If everybody just downvoted the questions they deem unworthy of an answer because "you are supposed to know this", there would be no questions answered.
I fail to see the "complaining" side in all of this. I interpreted the question as "what's the meaning behind 'm_'?". Until I read your comment, it didn't occur to me to see it in a complaining light, also because I imagine the question would have been worded differently in that case (e.g. "why m_ instead of <something_else>?").
And judging by other comments to the question, I wasn't alone, luckily.
This is the often case where some HN downvotes you to heck and the next hour bunch shoots you up to the moon. There is pretty much no predictability and it all depends on whether your responder makes a good case.
OP's approach works if all dudes involved read the code before chaning it and stay off private parts even if they aren't guarded by the language constructs.
That's not how software development works in e.g. companies where multiple people use the software (framework, libraries, API's) at different levels/roles.
For example, if we develop some framework to be used at multiple sites in the company, we can be 100% sure that at some point someone will assume that anything marked 'public' in the API is fair game to use (which IMO is a reasonable perspective). If they start using internal API's or depend on class internals that are only public just because the language did not provide us with a reasonable way to hide them, at some point we will change them and working systems could break elsewhere. In a similar fashion, misuse of private API's by violating preconditions that are completely opaque to users of your (intended) public API can lead to the worst kinds of bugs and unpredictable runtime failures.
Software development always seems so simple and pragmatic if you don't need to care about other people you have no control over.
... in your setup. All shops are different. To each their own. There's no private/protected scaffolding in the Linux kernel to give the most obvious example.
Well last time I checked the Linux kernel was C and not C++, so that might explain why ;-).
I don't know anything about kernel programming, but I assume that there's some very, very strict conventions about how the various parts of the Linux kernel interact, and nothing gets accepted for merging unless you follow them. This would be similar to imposing scope/visibility constraints at the language level, but considering C doesn't have anything for that, an extremely strict development process is the best you can do.
emm...
All we could say is that,
kernel programmers'/reviewers' technical background is much higher/better than average python programmers'/reviewers'.
Seen too many times these "Mr/Mrs. More or less" engineers simply modify the internal states as short cuts to compensate real engineering.
Equally being too defensive and over-complicating code in case a "bad" developer makes a breaking change often leads to code that is worse in the long term.
So it's an implicit contract based on syntax vs explicit?
It's a user/m_ prefix vs compiler/private. Im not sure if former is better. It's a strong trade off imho.
In my experience relying on people reading code and agreeing on imlicit contracts does not scale beyond 5 people, but maybe I've been mistreated by life.
No, not really. It works quite well for large projects too. Just look at any open source that is C-based.
The only mechanism for safe-guarding access to private parts is using privately defined structs that are passed around in a form of opaque pointers. Some project use it (PGPphone did, the original SSH did, etc.), some don't, e.g. BSD and Linux kernels.
It's really more of a matter of developers exercising some basic thought when using other people's code. And it just happens so that there's more of them outside of the C++ group than in it.
I come from an embedded development environment, and for me simplicity is everything. But even that, I think the approach of an "all public" class is a little bit risky, and can be a pain to debug if accidentally you change an internal state.
If a class is a black box, when you put your hand inside it, it becomes the "pain box" from the Bene Gesserit.
This style of development obviously will not scale to large, long-term C++ projects with many developers, as anyone who has worked on such projects knows all too well.
No disrespect to you, QuadrupleA, do whatever makes you the most productive, but I wouldn't want to work together with you in this way :)
Over a long enough time, every API will be called in every possible way. If an API misbehaves, some code somewhere will begin to rely on the specific kind of misbehavior. This is just the reality of publishing API's, so you deal with it and fix bugs as they are discovered.
If you make your entire class definition public in a large project, you will eventually have code poking at the internals of your class. I've seen this countless times, it's some kind of inevitable natural thing.
Developer scalability:
You know everything about how your code works, but I don't. A well-defined API with clear access rules gives me a fair chance to avoid using your class in ways you didn't intend.
In small-enough teams, or with small-enough projects, it's possible for everyone to understand (mostly) everything, but there's some magic number of concurrent developers where that privilege ends.
> I started doing everything all-public ala python (struct instead of class by default) with occasionally an m_ prefix for "intended to be private", and I've never looked back.
This is the sort of accidental complexity that access specifiers eliminated, but somehow you managed to reintroduce by trying to reinvent the wheel.
The 'm_' prefix only means "private member variable, don'access it" to you and you alone, and anyone who will stumble on your code will not be aware or obligated to follow your personal style. That would not be the case.if you simply used a basic feature of C++ that's been available since the time of "C with classes".
To me, you're the epitome of the main source of C++'s problems: programmers who failed to learn the basics of a language and instead decided to develop convoluted hacks to circumvent proper use of the language because they believe their new discovery is clever although in reality they only managed to needlessly add complexity to a code base while adding brittle and bug-prone code.
What else? Do away with memory deallocation because there's nothing of the sort in Python and your C++ code is packed with segfaults?
And by the way, the pimpl pattern is used to provide libraries a stable ABI in spite of what change might be done to the implementation. It is not a fad or a clever-looking trick.
Python has a well-established convention of _underscore for intended-to-be-private members, with no enforcement, and it works fine. Just document your conventions. The horrors of some other programmer using it wrong seem overblown to me.
In my IDE in c++ I can quickly refactor m_ to non-m to make it "semantically public" without having to edit header files, change struct packing order / ABI, etc. And most of the time I don't even bother with m_. It's one less thing to think about. I honestly don't think access specifiers are worth wasting mental cpu cycles on in most projects.
Besides, in c++ people can always cast your intricately-protected object to a void pointer and start manipulating it nefarioisly anyway. Just document and communicate, and write simple straightforward code.
I used pimpl at the time for faster recompiles when changing "private" stuff. Better build tools got me around that problem personally, but I'm sure it has it's uses.
Those access modifiers are there for a reason and they work very well in not just C++ in many other object oriented languages like C# and Java. So, using Python to justify that style of coding is a far cry from anything that is reasonable. If it work for you that's fine, but I don't think it's a good idea generally.
> Python has a well-established convention of _underscore for intended-to-be-private members, with no enforcement, and it works fine.
Python does not have access modifiers. C++ does. It makes absolutely no sense to write Python code in C++ while entirely oblivious to basic, age-old C++ features just because your background lies somewhere else and you failed or refused to learn even the basics.
And you know what actually works instead of just "working fine"? Having the compiler throw an error if a careless programmer tries to access private variables.
> In my IDE in c++ I can quickly refactor m_ to non-m to make it "semantically public" without having to edit header files, change struct packing order / ABI, etc.
Don't you understand that's completely irrelevant? Just set the private member as private and it does not matter at all which naming convention you follow. Don't you understand that having to do nothing is better than expecting team members to be prescient about your convoluted and absurd naming convention used to avoid best practices?
> Besides, in c++ people can always cast your intricately-protected object to a void pointer and start manipulating it nefarioisly anyway.
Don't you understand that't entirely irrelevant? You're trying to argue that it's ok to ignore best practices such as using member access specifyers, which actually get the compiler to validate the code, if you stick with naming conventions, but arguing that it's technically possible to circumvent compiler checks under some specific circumstances is an absurd statement. Think about it: if you feel that enforcing a coding style is enough to enforce private member access then don't you agree that the same code review that is supposed to enforce your personal style can quite easily catch your hypothetical casts?
You keep accusing me of "failing to understand" or "refusing to learn" the "best practices" of using access specifiers.
I understand them well, and I've experimented with their use in my own projects, and decided they're a minor complexity I don't need to bother with. And that decision has served me well.
Dogmatic adherence to "best practices" is silly, even when people can agree on what they are. Learn them, yes, evaluate them carefully, but ultimately choose what works best for your constraints. Including what your team's conventions are.
The programmers I've seen taking most about "best practices" generally don't think very critically about their approaches and go with dogmatic answers, often tying up their codebases in ridiculous complexity. E.g. the "best practice" now to make a website is a React.js SPA, and now every site out there takes 20 seconds to load with 10 little spinner icons while all the HTTP requests go out to their microservices on docker clusters, and everything requires a hundred-library-deep build stack.
I strongly agree with your sentiment that 'best practices' act as a replacement for critical thought, and lead to messy, overcomplicated and flawed architectures. Every "best practice" would be best delivered with a criteria list of when it's actually applicable, and a shortcomings list of situations where its not.
> and now every site out there takes 20 seconds to load with 10 little spinner icons while all the HTTP requests go out to their microservices on docker clusters, and everything requires a hundred-library-deep build stack
That’s just the cost of progress. Gotta break a few eggs to make an omelet. No pain, no gain. There are always casualties in war. Get with the times, grandpa.
Hello friends! Author here, nice to see so much discussion :)
I've spent most of my adult life working on large C++ codebases with public API's (most notably Qt and WebKit), which has led me to cultivate a defensive programming mindset. Someone mentioned Hyrum's law, which is 100% accurate in my experience.
I should be honest and admit that Badge<T> is not yet diligently applied everywhere in the Serenity codebase. I wanted to "feel it out" for a while first, but now I've decided I like it, so I'm applying it more and more going forward.
Oh and while I have your attention, I recently posted a May 2019 update video[1] on the Serenity OS project if you're curious how things are going :)
I like this idea, except for the fact that the extra parametrer pollutes the method signature and method calls, for something that really should only be a compile-time directive.
I'm actually curious why something like this has never been included as a language feature, it seems trivial to come up with a syntax for fine-grained access control, that is fully backwards-compatible and a true zero-cost abstraction.
Most obvious first shot:
class A
{
private:
void badgedFunction() const friend Device, SomeOtherClass;
};
Did you ever check with the C++ working group if there are any proposals for language extensions for this purpose?
I've only just decided that I enjoy using this pattern so my internal standards committee hadn't progressed to the proposal stage yet. :)
Your syntax makes perfect sense to me, and I think it would make a great addition to the language. I wonder what the real working group would say. (I tried quickly googling for something that might sound like an existing proposal but came up empty.)
Personally I'd move the register_device() and unregister_device() functions outside of the VFS class entirely, perhaps to namespace scope.
Alternatively, there are other ways to leverage friendship and access control in C++. Here's one option:
template <typename Registry>
class DeviceManager;
class Device {
template<typename Registry> friend class DeviceManager;
int y;
};
class VFS {
friend struct DeviceManager<VFS>;
int x;
};
template <typename Registry>
struct DeviceManager {
static void register_device (Registry& registry, Device& device) {
registry.x = 1; // accessing privates
device.y = 2; // accessing privates
}
};
int main() {
VFS vfs;
Device dev;
DeviceManager<VFS>::register_device (vfs, dev);
}
Here all DeviceManager<>'s can reach inside Devices, but only the VFS device manager can reach inside VFS.
This is also flexible. Remove 'static' and you get yourself a stateful mediator/manager. Add a variadic template register_devices_s_ member to Devicemanager and you've got yourself a convenience function for registering multiple devices (of heterogeneous types) without bloating the VFS API and, without runtime cost, and without losing type safety (by e.g. casting down to Device& from USBDevice&).
The thing is, there is no restriction on who calls DeviceManager<VFS>::register_device. main() is able to do it.
This doesn't seem so different from:
class A { private: friend void foo(A &, B &); int x; };
class B { private: friend void foo(A &, B &); int y; };
void foo(A &a, B &b)
{
a.x = 1;
b.y = 2;
}
int main() {
A a; B b;
foo(a, b);
}
We've just obfuscated it a bit by using a template class with one function in it, and declaring that to be the friend instead of just a non-member function.
Yes, your version is what I mean by moving it to namespace scope, and it's probably what I'd do to start off with.
> there is no restriction on who calls DeviceManager<VFS>::register_device
Well, you can make it private inside DeviceManager and give the class its own friends :-))
Another benefit of my solution is that adding things to DeviceManager<VFS> doesn't require changing the VFS.h or Device.h headers at all. Anything that links these 2 classes is an entirely separate concern and very much in line with thinking in terms of models or concepts rather than types. What you want at the end of the day is an absolute bare minimum of member functions in any class.
To be honest, whatever the solution, the key is that having the Device class self-register to the VFS in its constructor (as in Andreas's blog post) is, imho, an anti-pattern and is what is really leading to this Badge mess.
Love this. I also felt that having an explicit `register` on a mediator like `DeviceManager` made much more sense for SOC, but this is really elegant. Maybe you should write and publish some C++ Idioms :)
Could you make the badge the last argument and give it a default value, so you don't need the initialization at the function call? It looks a bit strange and would need an explanation for why the empty initialization list is there IMO.
I'm not sure if it would work, cppreference has this to say about default arguments:
> The names used in the default arguments are looked up, checked for accessibility, and bound at the point of declaration, but are executed at the point of the function call
It's an empty struct so the compiler will elide any storage either way, but agreed having it be the first arg is a bit odd. Perhaps it's for regularity in the cases where you want to make variadic functions 'badged' in this way.
I'm surprised, particularly about the cross-linking bit as that seems to violate the spec (per my recollection of the spec, which is the most likely thing to be wrong).
It's worth noting that it does apply to the latest gcc and Clang but not to all the compilers listed there. For an example of a difference, the latest djggp (7.2.0) inserts an extra push in the call when the empty argument is present.
If you use a calling convention that mandates the use of stack, you would see that push, or at least have a byte reserved.
AFAICT the C++ standard itself does not specify the particulars of calling convention, which is a function of the platform, compiler and flags/attributes.
I do C++ for almost 20 years but never came across this simple technique
before, so I'm quite surprised that I actually like it.
I usually avoid 'friend' in these situations and just write
//private:
before the method declaration. But it actually happened that co-workers
used these private methods because they use code-completion instead of
reading the header...
But I might would call the class Friend<T> or something.
> But it actually happened that co-workers used these private methods because they use code-completion instead of reading the header...
A nice feature I've noticed in Elixir is that giving a function a "@doc false" annotation will actually prevent IDE/REPL autocomplete from completing the function. (It's still there to call if you type it yourself.) Maybe that's something C++ tooling could copy.
I like it but is access restriction really a problem? In my years of C++ programming it always seemed to be a theoretical problem more than something that leads to crashes.
This solves a chronic architectural maintenance problem in complex C++ code bases, which I've also run into countless times.
Any visible interface, public or private (via friend), will eventually be used by other programmers for other than its intended use case simply because it is there and accessible. For the maintainer of an interface with a single intended purpose, what should be a clean, tidy modification within the scope of its intended use ends up breaking all kinds of unrelated code that the maintainer wasn't even aware of that is using the interface in ways the designer didn't anticipate. It is a common form of architectural rot. Reducing the incidence of this in large code bases is usually enforced by soft means, like sternly worded comments, vigorous policing, etc but that only goes so far. Ideally, an access policy would be in the code itself and enforced by the compiler.
The public/private labels don't address this. If you make things public, the world has the ability to do unseemly things with your interface. If you friend a bunch of classes so they can access private methods, those classes have the ability to do unseemly things with your private implementation details. Both cases embrittle the architecture. What you really want is fine-grained ACLs to class members to document both intent and dependencies.
This method of restricting usage of methods to specific classes at compile time is clever and provides good documentation of design intent, I haven't seen that pattern before. I also would guess that the compiler elides the Badge argument since it is never used.
> With a sufficient number of users of an API, it does not matter what you promise in the contract: all observable behaviors of your system will be depended on by somebody.
The client executables have no idea how big a foo object is, or what alignment requirements are, let alone what its members might be. The client executables don't control the allocation, construction, clean-up, freeing, or anything else.
The programmers who use this cannot just edit a header file to gain access to something; the don't have a header file which tells them anything. They could reverse engineer the data structure, but to thwart that, you could scramble the order of the structure members with each new release of the component, so their hack wouldn't be backward or forward compatible.
It solves the problem by decoupling the public interface and necessary implementation details (object size), the register/unregister functions can be moved to an internal only header file, maybe even some file scoped functions. AFAIK this is not possible with a class declaration.
I think you could also solve this specific problem with an internal header file in c++ too (by moving register/deregister to it's own class) but this solution solves other problems like binary compatibility.
Problem is, nothing elides the presence of these obtrusive badge parameters in the source code.
A few bytes of stack is the least of my concerns on some file system device registration function that is called five times when the system boots.
If we imagine a software organization "going to town" with this badge approach so that there are badges all over the code base, it's not hard to imagine how it would be a nuisance.
Depending on how you approach it (eg if you don’t instantiate it as {} but instead something like access::device, which is a type alias for Badge<Device>) then I find being explicit about access rights is quite helpful sometimes. In this case, I probably would avoid it too in favour of private headers, but then, I prefer freestanding namespaces functions over objects when possible anyway (too much functional programming maybe), so I’d partition my API that way instead.
This pattern is defensive by nature so people who use your library in the future will be less prone to making mistakes. Also, it shows intent to protect that asset clearly which generally leads to easier maintainability. If someone sees this guys code, they will definitely ask questions about why he is doing it, and if they are competent, they can draw the same conclusions that his blog leads you to without having to read the blog.
I'd guess it's more architectural concern than runtime. Making sure only code that is supposed to uses a semi-private interface so people don't build dependencies that aren't wanted.
The C++ access restrictions exist solely to teach and enforce proper API usage by other developers.
They're supposed to trigger a reconsideration of the API when a new developer runs into a restriction, but more often in my experience the new developer will just add a new public interface.
It's still an honor system. There's a way to generate code that will call the "private" or "protected" data/function even if it violates what's specified in the headers.
A different pattern I have thought of would be to provide an accessor interface object, and certain functionality is limited to be accessible only through that object. Controlling who has access to that interface can be flexible (possibly returning it along with construction, or passing it in at creation time...) Albeit, this is not without overhead, and for that, Badge is actually better, in terms of performance, if it fits the use case.
What happens when you have multiple Device objects though, in this case, they can all access VFS..
--
C++'s private inheritance can be used here to avoid some of the gymnastics, wherein, that private aspect of the class can be passed out into the appropriate party at creation time or otherwise.
If it is, it's the first time I hear about it. It always gives me a kick when I make up a similar design pattern to some grey beard's invention. Alas, I should read up on those patterns to avoid reinventing stuff and sharing things with people with appropriate names.
I like it! The self-documentation of where the call is coming-from is quite nice (though a more descriptive name like CallFrom<T> would make that even clearer, were it the principle intent). I can't help myself, though:
badge.cc: In instantiation of ‘Badge<T> FakeBadge() [with T = Device]’:
badge.cc:25:38: required from here
badge.cc:10:10: error: invalid cast from type ‘FakeBadge() [with T = Device]::Stub’ to type ‘Badge<Device>’
return reinterpret_cast<Badge<T>>(Stub());
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A reinterpret_cast won't turn an instance of the Stub local class into a Badge<Device>.
That doesn't even have to do with friendship/protection. If we move that line into the Device class, the reinterpret_cast still doesn't compile.
Well, that's a problem in the C++ type system, isn't it; because it (effectively) constructs a value of type Badge<Device>, even though it's not a piece of code sitting in the Device class scope.
The rule should be that if the default constructor of T is not accessible in the scope, then any cast to T * or T & requires a diagnostic. Or something like that.
If I can allocate a suitably aligned buffer of sizeof (T) zeros, and then treat that as a T, I've effectively constructed a T.
No, that's a good feature of the C++ type system, because it (effectively) allows the programmer to construct a value of type Badge<Device>, even if Device thinks it's okay to prevent that.
It would be better to do eg "return Device::private:get_badge()", but C++ defectively doesn't support that.
Yes, in the standard it isn't defined, but on most--or maybe even all?--systems and compilers it's perfectly defined, since there is no reason why a compiler would layout two different empty structs in different ways; they're both empty anyways.
Edit: Actually, from reading some of the other comments in the thread, the standard apparently specifies that empty structs are 1 byte in size, so technically they would both be the same size, so casting between them should be fine.
It's not defined on systems unless a document spells it out. However, the actual behavior can be deduced to be harmless from black-box testing. compiler source code and generate object code.
Problem is that it doesn't require a diagnostic, and the actual behavior of casting one empty struct foo {} to another empty struct bar {} is likely harmless and portable.
Oh wow, that's so similar! So similar in fact, that I must have seen this blog post at some point and internalized the idea :)
The improvement that Badge<T> makes is the <T> part; you don't need to look up the PassKey declaration to see who can construct PassKeys, it's encoded right there in the type instead.
I do like how PassKey makes the copy ctor private. I think we can improve on that and delete both the copy and move ctors in Badge.
Is it possible to have a proper compile time ACL for C++?
Arguably the Badge pattern as presented in the article is just an honor system that provides zero security, since it's trivial to forge a fake Badge: https://repl.it/repls/BountifulQuerulousCron
Of course, type systems are orthogonal to any real security, at least in most languages, and certainly all languages with no-holds-barred interface to machine code.
This is especially helpful in conjunction with private constructors and make_shared or make_unique. You cannot even friend make_shared because of namespace aliasing in standard libraries, but shared_from_this can make improperly allocated objects error at runtime. Private badges/tokens solve that problem by allowing public constructors requiring a parameter value of the badge/token type that has limited visibility, but can be constructed inside a factory and passed to make_shared.
C++ could solve this without badges, if it didn't have a very tiny, silly misfeature.
The misfeature is this: a class A can only declare a specific member function of class B as a friend, if that B member function is public!
Example:
The idea here is that Device has a static member function called Device::registrationHelper. That specific function (not the entire Device class) is declared a friend to VFS, and so that function can call VFS::registerDevice.
[Edit: this doesn't quite match the Badge solution, though. If this worked, it would have the problem that the registrationHelper has access to all of VFS, which is almost as bad as the entire Device class being a friend of VFS. That is one of the problems which motivate Badge. Nice try, though! The C++ restriction is probably worth fixing anyway. What the C++ friendship mechanism really needs here is to be able to say that a specific member function in B is allowed to access a specific member of A.]
class Device;
class VFS;
class Device {
public: // we want this private!
static void registrationHelper(VFS &v, Device &d);
private:
void registerWith(VFS &vfs) { registrationHelper(vfs, *this); }
};
class VFS {
private:
void registerDevice(const Device &);
friend void Device::registrationHelper(VFS &, Device &);
};
void Device::registrationHelper(VFS &v, Device &d)
{
v.registerDevice(d);
}
But we are forced to make Device::registrationHelper public, which defeats the purpose: anyone can call it and use it is a utility to register devices with VFSs.
If we make Device::registrationHelper private to prevent this, then the "friend void Device::registrationHelper" declaration in VFS fails.
This is an oversight; the privacy of the Device::registrationHelper identifier means that the VFS class is not even allowed to mention its name for the sake of declaring it a friend.
That should be fixed in C++. Declaring a function a friend shouldn't require it to be accessible; we are not lifting its address or calling it; we are just saying that it can call us. Allowing a function to call us is not a form of access to that function, yet is being prevented by access specifiers.
I might be wrong, but doesn't it look like a way to enforce SRP violation? In the provided example we don't only let the device know how to register itself via VFS, but we also make sure there will be no future DeviceManager to do that. Unless we duplicate the entire interface with Badge<DeviceManager> tag. Am I missing any non-obvious benefits of this approach?
Sometimes, clever little patterns like that are useful to get stuff done in time. However, it that case it definitely smells a lot. Also, the global singleton stinks.
To me this is a violation of the Interface Segregation Principle. I think the author went the right way to fix it, but not far enough.
He added a parameter to segregate the device registration operations from other operations. It works, but the segregation is artificial : you still need a Device object (which seems to be used by a lot of classes, another smell, god object ?)
Instead of asking for an entire Device object, register_device should ask for a different type, specific to that operation, like « NewDevice », « UnregisteredDevice », etc. This object would depend on very specific information only available at the device creation. Now, if another developer tried to call register_device, he would need to create a weird object that he never heard of, with parameters he cannot even provide.
Is an object really a god object if lots of people use it? I thought it was the opposite way around: a large object that contains too much functionality, but this device could be a tiny single purpose object for all we know, that a lot of clients need to use.
In D, you can access private members of a class within a single module. I like it, because it allows to move methods outside of the class, but doesn't require explicit friend declarations. And you still have the data protection because other modules can't access the private fields.
I had never seen this before, I cannot think of another way to do this. I suspect it has 1 byte overhead (on the stack) cost, but that's pretty cheap and may be optimized out.
While it makes no difference in this case, it's worth noting that Badge<T> is not an aggregate type because it has a user-provided constructor. Therefore the object is value initialized, i.e. the braces are equivalent to 'Badge<Device>()'.
edit: what this means is that even if the struct had data members, you couldn't brace initialize it like an aggregate type, and to achieve something syntactically similar, you would need a suitable constructor.
This is essentially a capability system enforced by the compiler, which means your code is not actually in control for any third party caller.
A glaring security hole. Any old hacker can forge or clone a data structure. This "badge" (AKA token) has to be explicitly unpredictably replay-proof generated and hard to forge, and also automatically verified.
Seems more of a people problem than a software problem. That's normal. Why not just use a comment that says "Don't use this. It will break."? If people can't obey simple instructions, you have a different problem entirely.
Hmm, IMO Badge has two main advantages over your approach:
1. Aesthetics. (This is down to personal taste of course, but I much prefer how Badge gets the job done without needing multiple lines of code at every declaration.)
2. What if I need to put the function implementation out-of-line?
Wouldn't -Wextra yell about the Badge being an unused parameter in the register devices? You would have to do some kind of dummy call that the compiler would have to optimize away.
> These functions are called by all Device objects when they are constructed and destroyed respectively. They allow the VFS to keep track of the available device objects so they can be opened through files in the /dev directory.
> Now, nobody except Device should ever be calling VFS::register_device() or VFS::unregister_device(),
Why?
Either have VFS call the constructor because non-registered devices are banned and RAII is good, or don't create arbitrary restrictions to hamstring how the rest of the system manages Devices.
I kind of agree that it's not clear why a VFS needs to register devices that only register themselves, however, if you suspend disbelief on that one it's a clever trick that is easy to imagine being useful.
This badge thing is clever, but seems like yet more accidental/unnecessary complexity on top of the already-probably-unnecessary private/protected/friend - as requirements change you now have this extra layer of access-control cruft to reason about, that you probably wouldn't even miss if it didn't exist.
Not to get too ranty, and no disrespect to Andreas, but the C++ culture seems to thrive in unnecessary / accidental complexity, deep template hacks and the like that add friction to refactoring, obfuscate underlying logic so you miss opportunities to simplify, and turn straightforward programming problems into brain-bending template puzzles.
OOP also; I've seen so many crazy class architectures that could just be a handful of plain-old-functions, complex virutal "type families" that could just be a TypeOfThingThisIs enum with some if statements in the respective functions, etc.
I don't know the Serenity codebase and use-case very well, and perhaps this kind of thing has a place in large libraries, but honestly even then I'd lean towards just making everything simple & public and use a prefix when needed, or describe appropriate usage in documentation. Seems to work in the python world (minus a few outlier codebases that go OOP-crazy).