There are many reasons why global variables are bad, but extremely high on the list (if not first) is concurrency issues. The fact that the author think it's acceptable in any C or C++ code base to put in code like:
static int prv_counter;
int counter() { return ++prv_counter; }
Is insanity. Like, this is programmer malpractice. This function can only ever be called from one thread (not to mention: using `int` instead of `int64_t` is also a trivial mistake, this easily overflows). This is the kind of thing that enters a code base, works fine for long enough that everyone forgets about it, then causes horrible security issues and crashes. The idea of saying this is "good use" of a global variable is... this person should not be giving advice on good coding.
If you want to do this (and you shouldn't, because global state is bad for 14 other reasons), at the very least, make it thread safe and not trivially overflowing:
Except the author's example is single threaded, so for that specific case your implementation of counter is needlessly complex and would actually be confusing.
The point is that there is no solution that works for all use cases. If you always attempt to write fully generalized code like that you'll end up with tons of unnecessary complexity. Solve the problem at hand, not some hypothetical.
The author even specifies a rule that covers your case:
"If you're using threads, global and static variables should be thread local. If they are not then the discussion becomes about sharing data across threads and synchronization, which is a different topic."
I'm sorry, but no. This is one of the primary reasons why global variables is bad, because of concurrency issues. If you add this to a code base, are you guaranteeing that at no point in the next decade, NOBODY is going to add a second thread? What if this is a library? What if some other library you're using runs a thread?
Doing this is planting a time-bomb in your code-base. It's going to lay dormant for a long time, and then explode, and be impossible to debug. This is why mutating global variables is `unsafe` in Rust, which is the right call. There is literally no C or C++ code base where I would allow this to pass code review, regardless of how single-threaded the author thinks it is.
> needlessly complex
It looks more complex than it is because I set the memory order to relaxed as an optimization. You could also write this:
If this was a junior making this mistake, I would sit down and talk to them and patiently and explain the issues. If you're writing a blog post on the internet saying "this is a good way to write code, do it like this!", I'm sorry, but I'm going to be very harsh. We haven't even discussed using `int` and not properly scoping the static, also rookie mistakes. Someone is going to read this article, and think this kind of code is ok. It is not.
> "If you're using threads, global and static variables should be thread local. If they are not then the discussion becomes about sharing data across threads and synchronization, which is a different topic."
This is nonsense. First off all, a counter like this wouldn't work if it was a thread_local, because then you'd get duplicate values. If duplicate values are ok, why is it global state at all? The only reason you'd make it global state is to produce unique values for that process run. And the general sentiment is like saying "this is how you drive drunk safely. We're not discussing keeping control of your car and crashing into a tree, that's a different topic".
This IS why global variables are bad! Not the only reason, but a pretty darn big one!
Not all the work we do is as part of large teams or on multimillion loc codebases.
In your context what you say is definitely the correct take, but I stand by my take that there are relatively simple programs written by individuals/small teams that don't need to be complicated by corporate scale concerns.
The problem is however that the internalized advice is shortened to "don't use global variables" and not "avoid global mutable variables when using concurrency".
Constant global variables can be very useful. Mutable global variables can be totally fine in a singlethreaded (e.g. typical embedded) program. I agree that you should still use them with caution, but every advice that says: "don't do $X" should come with instructions under which circumstances it is valid and under which it isn't — and how to get the intended behavior instead with e.g. message queues, locks or whatnot.
The point is obviously that the counter is centralized, and it relates to the previous example where is no concurrency. The need for synchronization when sharing data across threads is mentioned just below that.
It kinda does but perhaps not entirely. The increment will always end up in the right place afterwards but internally if you expect it to be +1 in the same thread you’ll sometimes be wrong
No, this is absolutely not true at all. Calling this function from multiple threads is undefined behaviour in C++ (unless synchronized using some other mechanism), you get NO guarantees what so ever on program behaviour.
Best case scenario is that the loads and stores are interleaved, which leads to multiple threads returning the same value when calling counter(), which will guarantee crashes elsewhere in the program (the purpose of functions like these is to produce UNIQUE values, after all). But it's undefined behaviour in any case, it's just unacceptable to put in a C/C++ code base.
I'm the author of the article. How many comments are you going to write about how my usage is bad citing multi-threads when I said multi threading is out of scope? And how do you not understand this would be a how-to use thread locals correctly when you are dealing with multiple threads.
I'm sorry you feel offended, but you did write an article online with a deliberately provocative title. If you do that, you need to be able to deal with criticism. And not considering concurrency when mutating globals in C/C++ is not acceptable (never mind good) practice. You can say "threads are out of scope" till your blue in the face, but if you write an article with the thesis "globals are good, actually", you have to be able to deal with people saying "they're dangerous because of thread safety". That is a legitimate criticism of your thesis. In addition, my other criticisms of your code (overflow and properly scoping statics) have nothing to do with concurrency.
Has anyone told you to never use atomics? Have you not heard lockless programming is hard? (I saw this recently https://wiki.libsdl.org/SDL2/CategoryAtomic,) Have you written multi-threaded programs? I written two large ones. The fact you're suggesting non-experts use atomic is insanity. As well as criticizing 'overflow' in an article showing minimum easy to understand code to be read by people using completely different languages
I fail to see how does that relate to the article in question.
The author provided an example for which counter() works well, he didn't claim that the same "pattern" would be good for 100% of the use cases.