ScopedPointer has been deprecated for around a year now, with the recommendation to use std::unique_ptr instead. The XML breaking changes return std::unique_ptr instead of raw pointers and ScopedPointer doesn’t have a constructor for unique_ptr. If you find + replace ScopedPointer with unique_ptr, you should be fine.
thx, aware of this and yes, the changes aren’t too bad, but aren’t listed in the breaking changes file so I wasn’t sure whether they were intentional or not.
I got a couple of these too and it really should be listed in breaking changes. I also had to get rid of a delete instruction and rewrite some stuff (for the better) in old code, so exchanging ScopedPointer with unique_ptr does not solve everything.
It does seem like this has been a long time in the making as Jules change dates to October 2018?
Yeah, we’ve had deprecation warnings for a long time on ScopedPointer. The idea with these new changes is not that you’d change it to unique_ptr, but that you’d use auto, e.g.
if (auto xml = xml_file.getDocumentElement (false) )
and preferably switch to the new, even shorter syntax:
it’s not clear to me how to return the xml structure created by createXml() from a ValueTree.
Have a look to the following code:
std::unique_ptr<XmlElement> SWAM_Processor::getPluginStateXml()
{
auto state = pluginState->copyState();
auto xml = state.createXml(); // this is a std::unique_ptr<XmlElement>
return xml; // here xml has a correct structure
}
void myFoo()
{
getPluginStateXml().get(); // getPluginStateXml().get() has an empty structure!
}
I think createXml() only returns a nullptr if you give it an invalid ValueTree, but obviously you’ve not posted all your code so maybe this snippet isn’t the whole story.
It’d be easy to debug each stage and see what values are getting passed around, right?
In my humble opinion, this is a bad use of auto because this way it hides object live time. Writing out std::unique_ptr makes things much clearer. With auto and if you are used to these methods returning pointers, things will look very odd for people making the transition to the new XmlElement return types.
Of course I am against using lots of auto in general and I do think the Juce team is overdoing it quite a bit at the moment. I believe it’s going to bite you in the future as it tends to make things unclearer. My personal rule is to only use auto in cases where I otherwise would have to write types twice. It’s like you want to pretend C++ was dynamically typed, but it just isn’t.
To anyone who has an anti-auto attitude, I’d strongly recommend asking yourself why pretty much every C++ committee member and expert/teacher on the subject thinks the opposite to you. The people who’ve really deeply studied this stuff and know what they’re talking pretty much all recommend using it as much as possible.
When I first heard about ‘auto’, I also assumed it’d be a tool to use sparingly, but with years of personal experience using it in huge, diverse codebases, and seeing how valuable it is in refactoring and improving old code, I totally changed my mind on that.
You might also want to note that all the other modern statically typed languages that have sprung up in the last few years all encourage the use of the same pattern.
No… anyone who thinks for a second that ‘auto’ has anything to do with dynamism would be completely misunderstanding the purpose of it.
You make it sound like there’s a big agreement about auto amongst experts, but that is not true. Of course, everybody agrees auto is a good addition. However, the number of cases where its usage is beneficial is a matter of great debate. I realize we are at the opposite ends of the spectrum, I just wished Juce was somewhere in the middle.
For my daily work the impact has been that I now sometimes have to dig through the Juce code to figure out what types are used. Some of that comes from IDEs just not always being able to deduct the auto types for auto-complete. And life would be better if my IDE could display the type of an auto variable, but I don’t see that in XCode and VS.
I tend to look through all the Juce development commits quickly to see what is new and since the flood of auto, single commits (f.i. only containing changes inside one method) are a lot harder to understand as we lost local type context.
The lesson I learned is that if you rewrite something to remove the types, and then find that it’s less clear, then that’s a code-smell warning you about other problems… perhaps it needs better naming, or better structure, or maybe the types you’re using are poorly thought-out.
And that applies to juce too - if you see code that’s unintelligible then we should fix that, but in my experience the good fixes are never to add more type info, it’s to rephrase other aspects of the code.
I’m not going to get drawn into a debate here - there are countless great books and talks that make the case much better than I can do.
Right - but you should question why you need a raw pointer at all? Having to call release() is often a warning sign - if you’re refactoring to replace raw pointers with smart pointers, you should try to keep your objects in smart pointers for as long as possible, not just immediately pull them out and go back to old-fashioned raw pointers.
Hmm, it was documented as deprecated, maybe not physically marked in the code. (We actually haven’t ever marked any whole classes as deprecated, only functions - perhaps because it’s not possible on all compilers…)
just to mention it: another code snippet that won’t build with the last changes (and another reason to call release()) is ownedArray.add (XmlDocument::parse (file));
Maybe OwnedArray could have an overload for the add() method that accepts unique_ptr<ElementType>…? (And internally releases the passed in unique_ptr.)
