Do JUCE Devs have a C++ Style Guide/Coding Rules document?

Years ago I was working with another team and we were stuck on a bug for days. The problem turned out to be code that looked like this:

if (condition)
     doFirstConditionalThing();
     doSecondConditionalThing();

Of course the intent was:

if (condition)
{
     doFirstConditionalThing();
     doSecondConditionalThing();
}

The problem was that none of the human developers could see it. We all looked directly at it and our brains saw the indentation and filled in the nonexistent braces automatically.

Even after the bug was identified it was still difficult to persuade other developers that it was a problem! Even if I pointed at the exact line of code and explained the problem, it was still quite difficult to overcome the psychological barrier.

Now there’s the -Wmisleading-indentation warning; that would have been helpful back then.

Matt

2 Likes

Exactly! Another common mistake is to do the following:

if (condition)
     //doConditionalThing();  // commented out while debugging

doTheNextThing();

That debugging session isn’t going to go the way you think. :smiley:

And these kinds of issues aren’t purely academic. That’s why I mentioned goto fail. That was 100% the result of not using braces and the developers not seeing it (and the tools not reporting it) and it lead to a massive security vulnerability that went undetected for years.

Anyway, I won’t belabor the point. It’s an easy class of mistakes to avoid by typing two extra characters. (Most of the arguments I’ve seen for leaving out the braces are “Bro, I know what I’m doing.”)

2 Likes
#define LOG(x) printf(x); fflush(stdout)
if (condition)
    LOG("test"); // bro ..

Always use {}'s. Just do it.

if (true) { things(); } // this is fine
1 Like

well, that’s a horrible macro definition for several other reasons :wink:

1 Like

I said the JUCE style guide “recommends a few things that are generally considered to be bad” but I just read through it again and the single-line braces thing was really the only thing I could find.

Even though I dislike many of the stylistic choices (why put a space between a method name and the opening parenthesis?!), I do appreciate that the JUCE codebase consistently applies their own rules! It’s a lot easier to read than many other codebases out there…

4 Likes

well, that’s a horrible macro definition for several other reasons :wink:

Yeah, I’d consider it also a violation of the always use {} rule, as well.

Anyway, cool thread.

This has been a really interesting conversation. I was just looking at one of my older project and realizing that I could be more consistent. I have generally been moving towards the C++ Core Guidelines . Most of my industry experience the last many years have been Java, so it’s been a bit of a transition.

We have looked at using clang-format before, but it makes a real mess in a lot of places and no amount of configuring avoids this. We also don’t want to do more than one “reformat the world” commit if possible.

Ultimately I think we will move to some way of doing automatic formatting. Maybe clang-format has improved since we last gave it a go. It’s a surprisingly large bit of work though, so it’s difficult to justify over other large, or valuable, bits of work.

1 Like

Maybe GitHub - uncrustify/uncrustify: Code beautifier can do a better job? Haven’t tried it myself.

IMO messy auto-formatted code > tidy manually-formatted code. Would be interested in what specific areas you found were particularly messy with clang-format?

1 Like

I’ve just run into an issue - my Processor defines several Parameters in the constructor, which I’ve necessarily hand-crafted into single-line-per-parameter code statements - these can get very long, in terms of line-widths.

Well, I just tried to format some code (using gnu-indent, big mistake) in order to have a ‘standard format’ which can be used to diff another version of the same code, and in so doing have completely lost all the hand-formatting of the parameter list .. grr .. this is quite frustrating, as both versions of the code will now need to be hand-formatted .. again .. prior to continuing a diff.

So I’m all ears for a generalized clang-format string that can be used from now on, to ensure all future code gets formatted the same way for sanity ..

1 Like

Lambda expressions were particularly messy.

For JUCE the source code, including its formatting, is the product. Formatting matters more in this context than many others. We know that the current (and future!) state isn’t going to please everyone, but there are very few places in the codebase where things look completely mangled.

3 Likes

My 2 cents (which is worth exactly that) - I like the current formatting.

Rail

4 Likes