Why is AudioSampleBuffer Destructor now non-virtual??
https://github.com/julianstorer/JUCE/commit/dd1a3496c25d63e0bdf94a1b6f99cf4e58b9c960
These are the kind of changes which can cause many troubles, and result in a different runtime behavior (if you cast the type, desctructors won't be called) ,
Its better to rename a class instead, and mark the old class as depracted instead of changing this elementary behavior of a class,
grr... 
(searching a bug for several hours...)
can you make it virtual again?
No, I can't make it virtual! I was correcting a silly mistake, and it's embarrassing that it got accidently made virtual at some point - that should never have happened!
Having a virtual base class with no virtual methods makes no sense at all, especially when it has a whole heap of non-virtual methods. Think about it! Basic container classes like that one should never be inherited from - doing so is awful C++ design, even if you use non-virtual inheritance and don't attempt to pass around base-class pointers. It'd be like inheriting from String or Array - yuck! Always prefer composition over inheritance when building upon that kind of class.
But it was virtual, what does say to me as a programmer ist perfectly ok to subclass it.
And if your code depends on it, it changes the runtime behavior, when you make it non virtual. This is awful. Because you didn’t notice it, maybe your customer notice it first. This is awful, because it results in instabilty!!!
Please make it virtual again.
I'm genuinely sorry it was ever virtual, and sorry that my mistake led you into thinking it was ok to use it as a base class. And I wish I'd noticed it sooner. But no, of course I won't make it virtual again!
Like I said, it'd be horrific C++ style to take something like that (i.e. a virtual class with no virtual methods) and pass it around using a base class pointer.. Even if you had a class that was specifically designed to do that, it'd make no sense to do so, as all your subclass functionality would be inaccessible without down-casting the pointer somehow.. And AudioSampleBuffer has value semantics, so using it in a hierarchy would cause all kinds of slicing problems. Honestly, I dread to imagine what you must have be doing to get into a mess with it - this is probably a good excuse to have a serious think about your OO design!
this was an older class, i added a samplerate attribute (double) to the class, it would be nice if you could do that.
Or you implement an (scoped)pointer for a propertySet or something like this to store additional information.
But if i have to change it to a composited object, i near have to change millions lines of code (and this will introduce new errors), i want to keep all my products to be up to date to the latest juce code base.
Maybe it was not pure elegance OO design, but what i’ve done was 100% C++ standard. The problem is when ever you do such a dramatically API-Change, this can have an effect on user-code at runtime!!! (this is the point!!!) .
And this much much more awful…
Look, I've apologised about making the mistake in the first place, but am unapologetic about fixing it because I never dreamed that anyone would be crazy enough to have actually written code that used it as a base-class, when it's so obviously unsuitable for that!
(And I've just had a long and tiring discussion with someone else about why sample-rate (and other metadata) doesn't belong in that class, and don't feel like going through the same discussion again right now)..
And anyway, surely it'd be a trivial bit of find-and-replace to fix this - all you'd need to do is to make sure your derived class has a virtual destructor, then pass around pointers to that class instead of AudioSampleBuffer, and everything will work fine, even if you continue to (yuck) use AudioSampleBuffer as a base.
And anyway, surely it'd be a trivial bit of find-and-replace to fix this - all you'd need to do is to make sure your derived class has a virtual destructor, then pass around pointers to that class instead of AudioSampleBuffer
Mhhh this is even more bad, because it will not fetch any errors while compile time, when not casting to the derived type ;(
And adding a optional metadata by size of pointer would be the best solution, please!
....on classes without virtual methods!
It doesn't make any sense at all.
Just make your "derived" class *include* AudioSampleBuffer instead of inhering from it.
....on classes without virtual methods!
It doesn't make any sense at all.
Just make your "derived" class *include* AudioSampleBuffer instead of inhering from it.
+1!
chkn: sorry if you think I'm being a C++ style nazi and unsympathetic to what you're trying to do, but based on what you've said, it sounds like you must have a nasty mess there, and some refactoring would probably be wise. I've done the same kind of thing cleaning up the tracktion codebase, and trying to make things work with the least possible disturbance usually means compromises and makes your code less maintainable in the future. Recognise your design mistakes, and fix them, even if it's painful!
And asking for properties on the AudioSampleBuffer would be a violation of the single-responsibility principal - that class is not responsible for carrying around your other data. If you need both a buffer and a sample-rate then you should build a class that contains those two things, and pass that around, not try to hack the buffer to hold whatever it is you need to know. I had this exact same argument last week with someone else, and it seems crazy to me that people don't get this.. The AudioSampleBuffer's one and only responsibility is to hold some channels of audio data, not to tell you anything about that data, or where it came from, or how it should be used.
Create a class that contains AudioSampleBuffer and provides a conversion operator, so you can pass your class to any JUCE routine that expects the buffer. If you dont like having to access the ‘buffer’ data member every time, overload the member selection operator so you only have to change ‘.’ to ‘-.’:
struct CompositeSampleBuffer
{
double sampleRate;
AudioSampleBuffer buffer;
operator AudioSampleBuffer& () { return buffer; }
operator AudioSampleBuffer const& () const { return buffer; }
AudioSampleBuffer& operator-> () { return buffer; }
AudioSampleBuffer const& operator-> () const { return buffer; }
};Thanks Vinnie, but this wouldn't be an AudioSampleBuffer anymore, this is the problem
Doing re-factoring of an already ready and bug-free product is a mess, and makes no sence at all.
But i want to keep the code-base update, in case something will come up.
I completely agree would you say about virtual destructors, its allright!!!
But you have also the responsibility as a provider of a toolset for developers to provide a consistent api!!
And changing such things, like virtual keyword can have dramatically effects.
These are two aspects which need to be balanced.
So what things will happen if you add the virtual keyword to the destructor again?
So what things will happen if you add the virtual keyword to the destructor again?
My professional reputation would be damaged! It's already embarrassing that I made the error, and that I didn't notice it for so long!
Maybe you should just hack your copy of juce to make your stuff work, but I can't deliberately put something as broken as that into the codebase just because one user wrote some terrible code that relied on it!
But you have also the responsibility as a provider of a toolset for developers to provide a consistent api!!
I obviously never make changes that will break reasonable code, but like I said before, I never imagined that anyone would have been abusing it like you were, since there's no sensible use-case for anyone to ever do that.
one user wrote some terrible code ... anyone would have been abusing
would you please calm down, please?
When i have written this class, long time ago, i done it in the classic polymorphic way of adding porperties to an object.
And when it comes to polymorphism, an important thing is that virtual C++ destructor has a defined behavior, while using a non-virtual destructor often has an undefined behavior. And some part of philosophy is to write code "stable", and this is the point.
Introducing API-Changes "under the hood" the potentially introduce "undefined" behavior into a product is not good.
Even when they grow your reputation (sorry)
And anything i did, i've done relying on C++ Standard, maybe not 100% modern OO-Standards.
And releasing stable software is also good for somebodys reputation.
(i know about the "composition is better than inheriting-thing", and what Scott Meyer says about virtual destructors, and i follow it)