I just updated to the latest JUCE build - and my project fails to build as the JUCE::Image class is now declared as ‘final’. Is there any reason I am missing as to why this change has been made? I have a small family of derived classes that will not build unless I move that keyword.
[long discussion about C++ over time morphing towards PL/1 removed…]
It’s to tell you that you have made a code design mistake by inheriting from the class. (It sure can be annoying, I myself bumped into this recently when an external library wanted to inherit from Juce’s String. Of course that has also been marked “final”…Luckily the author of the library was able to change his code to not have to inherit from the Juce class.)
I get the properties of the ‘final’ keyword. However I am curious how come this is now a ‘bad idea’ all of a sudden. I can rewrite the class to delegate to an ‘Image’ member variable - however this breaks the elegance of the solution I had, and I cannot see the rationale for the change.
In short - ‘I have made a code design mistake’. How come it is a mistake? Apart from the presence of the keyword preventing me doing this? Why was the keyword added?
To make it clear the class was never intended to be inherited from.
Totally with you. My question is - ‘how come’?
Inheritance is a key property of OO design. Breaking it IMHO should only be done with very good reason. I would love to hear from the design team what the reason is?
I want to understand how come breaking inheritance here, and in general is a useful thing.
Juce’s Image is supposed to be used via value semantics, not via references/pointers. Value semantics and inheritance don’t play together too well.
Image by the way doesn’t even have a virtual destructor which should have been a telltale sign even before the “final” keyword was added that inheritance is not going to be a good idea.
The only argument I can see here is that the ImageCache model uses Image - and having derived classes of image might complicate matters. The code I have has been playing happily with JUCE for about 4 years… so…
Partly I am just annoyed that to comply with the library I now need to change a bunch of code I was not planning to change, re-test etc… - or do the short-term solution and comment out the keyword in the JUCE library until I have the schedule and testing time to go back and do this properly.
So - I am curious, genuinely - how come this got added now? Has something fundamental changed that makes this an essential fix, or is this just missing ‘decoration’ that was added to communicate the designers’ intention?
In most circumstances you should prefer composition over inheritance, however if there is a valid argument Jules has said he’s open to dropping the final keyword in some places.
It is a friday evening. It has been a long week. I am going there.
<rant>
So - we have a library of useful well-designed stuff. How it is designed internally is up to the designer. I am happy with the result and what I know of the internals gives me confidence in the competence and design consistency of the whole. It is why I am using it.
We have a programming language that (being C++) offers everything-including-the-kitchen-sink in the way of language features. I use a carefully chosen subset to keep code readable and efficient.
And we have a design principle, favoured in some cases but by no means all, that composition offers more design flexibility than inheritance.
And we have a keyword that ‘enforces’ the design dogma at the interface. (If you are going to play with us, you will follow what we have decided is best practice with our classes).
I get Jules’ comment above is a bit tongue-in-cheek (‘silly’, ‘beginner’ etc…) however there is a sense that this is either a) fixing a problem that does not exist, or b) promoting a dogmatic approach, not enforced by the language, into users’ code. (There is also incidentally an compiler optimization argument that I have not seen convincing evidence for.)
There is IMHO a fine line between utility and sticking your foot out and taking careful aim with a shotgun.
I resent arbitrary restriction on potential design solutions. I doubly resent re-writing functional, tested code for non-functional reasons.
<\rant>
I will go fix the class. In this case it is moot; the composition variation adds 40 lines of code with changes over the inheritance solution. I will swallow it.
I’d be interested to see an example of what you’re doing - since the changes related to the final keyword are open to debate and potential change if a strong enough use case is found (as has been said). Not to mention that’s probably what @jules will need to see in order to sway his opinion. There’s a good chance we will all learn something!
I’d love to have added it years ago, but ‘final’ is a C++11 feature so we had to wait until everyone had compiler support for it.
And seriously, of all the classes I added it to, Image is one of the ones where there’s no debate at all to be had!
If you don’t understand why all the C++ and OO gurus go to so much effort to give countless conference talks and blog posts explaining the whole composition vs inheritance thing, then you should probably watch a few! They’ll explain the rationale way better than I could in a brief reply here.
But the TL;DR is that the rules are that you inherit from a class when:
Image isn’t the type of class that would fit either of those categories, but as others said above, if you really genuinely think you have a use-case for inheriting it, I’d love to see it!
Hi Jules et al.
This has been an interesting exercise. Firstly just getting over my friday afternoon ‘aargh…’, and secondly looking further at that piece of my design.
I put this code in place a few JUCE versions back. The code that caused the issue was very simple; a local _SafeImage class that inherited from Image:: and ReferenceCountedObject:: . Within this code I am creating images (drawing onto images before rendering to the screen - these are condensed from a lot of data) which I then cached and managed directly. The images could, in the initial design, be very large (600,000pts by 20pts). In the new model they are somewhat more modest (up to 4,000pts by 20pts).
It may have been when I designed this I missed the ‘::addImageToCache’ method, or otherwise (in the dim distant memory) chose to use my own cache management due to my concerns about needing to actively control runtime memory footprint in a very memory-intensive application.
So - if I go via the ‘composition’ (aka. delegation) model - following the design pattern I have used would be a pain - with lots of forwarding methods. The alternative is to re-design using the ImageCache and embed Image objects rather than my _SafeImage reference counted pointers. My guess is that this should work fine. It will take time and require re-testing. I have faith ImageCache does what it says on the tin, and I can use releaseUnusedImages and setCacheTimeout to keep things clean and bounded.
So - the practical side will be dealt with.
The philosophical side here is more interesting; a simple design freedom is curtailed. Yes - this does promote a particular style of re-use which has its advantages. However its ‘proscriptiveness’ has removed a possibility for re-use that I made good use of in well-proven code. Unless something ‘dangerous’ is done inside Image:: that could cause unexpected runtime issues…
Interested in your thoughts.
DonBtw are you familiar with the -Dfinal= compiler flag? 
Make It Work™ instead of Make It Right™!
Yeah, there’s a list -Dprivate=public, and maybe -Dconst= …? 
I might search github for those…
-Dtrue=false and -Dfalse=true are obvious ones
Ouch! That’s a great example of exactly the kind of thing we were hoping to help prevent!
All the examples I’ve seen of people who’ve been burned by this change seem to either be because of a misunderstanding (like not realising that Image is already ref-counted and doesn’t need this extra layer of wrapper class), or where they don’t think the safety of composition is worth the extra effort of typing the extra member variable name when they access it (or adding wrapper methods).
But as I’ve said, we’re still waiting to see a killer example that’ll convince us to go back on this!
That’s gonna be hard to find an example, when everything is already delegated… drawing contexts, pixeldata, loading and saving… what else would an image need? Nothing happens in the image class 
Glad to oblige!
So - being a smart guy, who knows what he is doing, who missed this nugget when I first coded this solution, I am wondering how this design wisdom could be hilighted in the documentation for the Image class?
Thanks for the input. Back on the train.
