JUCE_DECLARE_NON_COPYABLE should be used under public access

General JUCE discussion
1

JUCE_DECLARE_NON_COPYABLE should be used under public access since it’s part of the class’s interface. Besides being better theoretically, it also results in better error messages.
See

There’s seems to be at least some consensus on this (e.g. https://abseil.io/tips/143 , https://stackoverflow.com/questions/18931133/must-a-deleted-constructor-be-private )

There’s a wrinkle: in JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR the leak detector is obviously logically private. I suggest just deprecating that macro and start using the JUCE_LEAK_DETECTOR macro separately more, under private access.

How about changing this in JUCE’s classes’ definitions?

2 Likes
2

I have JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR in hundreds of classes, and have never had a problem with the error messages. I also haven’t had any other issues with it, and not worried about what’s better theoretically :slightly_smiling_face:
Always having to add two separate macros in different parts of the class would be annoying, and more boilerplate. With the chance of forgetting one of the two etc.

I think it’s good that there are the separate versions (JUCE_DECLARE_NON_COPYABLE and JUCE_LEAK_DETECTOR). You can already use the two separate macros if you prefer (as you’ve noted), but I’m strongly against deprecating the combined macro.

3

It could be fixed without having two separate macros by adding a public: before the Constructor() = delete; and a private: before the leak detector.
That way it is irrelevant in which visibility that macro is placed (but not what occurs after the macro).

I don’t know if it’s worth the changes, since some codes might suffer from that visibility change, if the macro is placed somewhere in the middle of a block.

4

This will work if it’s always at the end

  • If not at the end, it will affect the visibility of what’s after it
  • This may be misleading if someone looks at the visibilty and assume from it that it’s under private
1 Like
5

Yes, that’s what I meant with

Seems like it can only be handled gracefully in each ones codebase by using the JUCE_LEAK_DETECTOR in private and JUCE_DECLARE_NON_COPYABLE in public.
Anything else will be a compromise.

1 Like