Usage of auto in JUCE code


#1

I noticed in recent posts that auto is used to replace rather simple types, for example in this commit:
`- const String faceStyle (font.getTypefaceStyle());

  • auto faceStyle = font.getTypefaceStyle();`

`- float w = getTypeface()->getStringWidth (text);;

  • auto w = getTypeface()->getStringWidth (text);`

In both cases it isn’t clear to the user (at least the ones not too familiar with JUCE) what type it is and in the first case it also removes constness. The saving of typing on the other hand is rather small, especially if the const String in the first place would be replaced by const auto. While I like the general JUCE coding style very much I’m not so enthusiastic about this. Maybe you can elaborate why I’m horribly wrong here. :slight_smile:

The JUCE coding standards also don’t comment on the auto keyword.


#2

There’s a few reasons outlined here:


#3

ckhf, thanks for the thread! I’m also a bit concerned about the increasing usage of auto in JUCE. In my opinion, using it for basic types just makes the code harder to read for people not familiar with everything on the level the developers are. If I hit asserts or problems in the JUCE library code in an “auto”-ized section I now have a harder time to understand things. If intellisense and xcode code-sense would always work, things would be better, but relying on those just seems inconvenient (also UI-wise).

I read the blog post linked above and it is not very convincing to me. It also suggests to get rid of my readability issue with the syntax auto x = type{ expr }; which is not used in JUCE as far as I know. In fact I like the (negative) comments below the blog much better than all the explanations why it’s good to use auto as much as possible.

My personal opinion is to use auto just when things become unreadable otherwise. This mostly happens to me when using templates, lambdas, some iterator types or stuff with deep nested namespaces, when types become unreadable.

Overall it would be great to have a section about auto in your JUCE coding standards and I hope this will become an interesting thread. Maybe it’ll even teach me to use auto more ;).


#4

When I first heard of auto, I also thought that over-using it would remove some readability from the code, but having actually used it for a while now, I changed my mind on that one.

I found that in practice it helps to strip out a lot of clutter and repetition. Sure, there are a few places where you’d choose to keep the explicit type because it made things clearer, but overall I’ve found that’s pretty rare.


#5

For now it surely generates a lot of commit noise. I tried using auto myself when it first got usable and then decided only to use it rarely, but what really made me dislike it was reading through some 3rd party code with lots of auto usage. I ended up replacing the autos with the concrete types myself so I could understand the code. Once it’s auto all over the place, everything just looks the same and gets very confusing. It can feel like the coder was just too lazy to make things concrete and readable.


#6

talking about coding style and auto, I noticed that you adopted auto* rather than auto& when dealing with ScopedPointer/SharedObject, while I used to do the opposite. Was just wondering if there’s any particular reason for that?
for example :

auto* list = p->mouseListeners.get();

instead of

auto& list = p->mouseListeners;


#7

Thanks for the link. Like pflugshaupt, I’m not totally convinced yet but maybe it needs to settle a bit.

One other thing: Although it’s not in the coding standards, JUCE used to follow the “if you can make it const do it” rule rather strictly. Any reasons why this is ommited when using auto?


#8

Holding onto raw references to smart-pointers is generally a bad idea unless you really need to do so (e.g. to assign to it or to call methods on the smart-pointer itself).

The danger of holding a reference is that in the code that follows, you could call something which indirectly modifies the original site of the smart-pointer, so that the reference becomes dangling.


#9

In that case the solution used in STL is the only safe IMHO:
If you create a reference to a std::unique_ptr, you get a std::shared_ptr which keeps the object alife until the reference is also freed.
Is that possible with juce::ScopedPointer too? This would avoid these kind of uncertainity…

For the original topic: I prefer also explicit typing, but I would not claim it is wise neither to use it always, nor use it never… just think a second of a reader who is not yourself, and you will know which to use…


#10

Yeah, we still do like that rule, though I’ve been finding myself favouring less verbosity recently, especially where the scope is small and it’s easy to see what’s going on. Still a good idea to use const in possible for variables that are used heavily in a large code section.


#11

No… a reference to a std::unique_ptr is a std::unique_ptr&. It’s just a class, there’s nothing special about it.


#12

My bad, I mixed it with std::weak_ptr.
Still, this is a great approach to avoid a pointer becoming invalid while being used in another place…


#13

Well, weak_ptr and shared_ptr work together, but they have nothing to do with unique_ptr, which is what the OP was talking about. That wouldn’t help in the case he mentioned above.


#14

I’ll add myself to the list of people who feels that auto is being overused in recent commits. What is this, PHP? :grin:


#15

Whilst I’m not a huge fan of the AAA approach (there’s an interesting talk here https://www.youtube.com/watch?v=ZCGyvPDM0YY which shows how AAA can be taken to extremes and feels like a bit of an abuse to me…), I would say that you do get used to its use.

There’s a bigger and bigger move towards coding styles where you don’t explicitly type types these days (take a look at Swift’s var and let) and it generally leads to cleaner, more terse code. In my opinion it leads to improvements in method, function and variable names as they’re where you get your context from and the bits you reuse rather than the variable declaration.

It also means if you change the types returned from methods and you’ve programed against the interface rather than the type you shouldn’t have to change much.

Having said that, it does always feel a bit wrong to use auto for primitive types as you don’t save much (but then does enforce correct use of literal suffixes which I do like).

The real danger can however come from mixing old, pre-C++11 code with auto. There are some very serious pitfalls with smart pointers you can fall in to so I always recommend using auto* if you know the object is going to be a pointer.


#16

And I thought I was the only old fart C purist here :smiley:

Rail


#17

This is what I’ve found too.

Another observation I found was that when you look at some code with an ‘auto’ and find yourself thinking “I wonder what type that is”, that’s often a sign that the code has other problems, usually needing better names or to be decomposed into more functions.


#18

I’m not a fan of auto either, because C++ is a statically typed language after all. Heavy use of auto seems like an attempt to make it look more dynamic with no real advantage other than typing less. (Having the IDE replace ‘auto’ by the correct class name would be great, though).

C++ isn’t dynamic and polymorphic in a way that you could drag a class to another superclass and be done with it without changing any code (as is possible with other languages). So why pretend?

Granted, working for decades with dynamic languages, I became a very lazy coder. Still, I have no issues figuring out a particular class, even more so as this is often a helpful hint at what I’m still getting wrong (if the expected class doesn’t compile). Using auto takes away this important feature of a statically typed language, namely the stumbling across misconceptions at compile time already.


#19

This is not the goal of auto but rather a good way to avoid typos and errors.
Want to know what type it really is, just use the tooltip in VS or from your favorite editor which should have this feature if he doesn’t yet.


#20

Well, I’m not a purist in the sense that I see all new features as “bad”.
For example, I like how range-based for makes syntax much cleaner, and I find myself using it in more and more places where I need to iterate over a collection.

Granted, I wouldn’t “transform” a particular algorithm I have in mind with the sole goal of using range-based loops, nor I would go back into code that I already wrote only to replace existing for with range-based ones, even if iterating over a collection.

If it ain’t broke, don’t fix it.

Sadly, that seems what is happening in JUCE with recent commits: blanket replacements with “auto” even in spots that are distant from the code that is the “centre of action” of the commit.
IMHO that only:

  • adds noise to commits in unnecessary places.
  • obfuscates types that were already well specified and thus didn’t need the “typo saving” balm that is “auto” in some cases.