Jump to content
Sign in to follow this  
Valik

Why global variables are bad and you shouldn't use them.

Recommended Posts

Valik

Global variables can lead to shit like this. I was doing work to fix an issue with AutoIt and Aut2Exe which contained two separately maintained (and nearly identical) lists of function names. Have you ever wondered why we would occasionally put out a release (like 3.3.5.0) that couldn't compile a function but it could run it? It's because we forgot to update Aut2Exe. The solution I came up with also solved an additional issue. It turns out that in order to use array initializer syntax we were creating a temporary local variable then copying it into a heap-allocated variable. A nasty performance hit (nearly 400 element array with 16 - 24 bytes per element (32-bit, 64-bit sizes)) that was necessary because any other way of creating such a massive structure would just be grotesque. I re-wrote the code to take advantage of static class members and the fact that they can still be declared with array initializer syntax. The class was designed to be inherited from and required near zero changes to existing code to just magically replace the previous list (protected members with identical names). Everything looked fine (and it was fine) until I ran it. The problem was, it crashed. Very strangely. Turns out that the global class that inherited from the new class was being constructed and attempting to use the function list before the list was initialized. In an application that doesn't abuse global variables this would never happen. The static member would be initialized before main() is ever called and all would be well. Static and global variables are are initialized in the order they are encountered. I had a 50-50 shot to get lucky and I didn't. So I had to re-write the class and turn it into a pseudo-container making use of local static variables. I spent a lot of time just re-writing the class. Did I mention it uses templates and macros (together)? Yeah, that's a mess. Compiler bitches and you don't even get an accurate idea what the error is because it's complaining about some template issue hidden behind a macro. I digress...

This story also involves Aut2Exe. Obviously my class was designed to be used in Aut2Exe as well since the whole point was to provide a single list instead of two lists to maintain. I made the appropriate changes to Aut2Exe, built it and tried to run it. And it crashed. This time it didn't even make it to my code. No, it turns out Aut2Exe had the exact same issue before I ever touched it. For years now Aut2Exe has worked by pure chance. That 50-50 chances of working I mentioned I had with AutoIt? Well, Aut2Exe has been living on that for years. I have no idea why it suddenly decided to break when I was working on unrelated parts of the code. It baffles me why it decided tonight was the night it wanted to dine in hell. But there you go, better tonight when the problem is fresh in my mind than later when I've forgotten about it... again.

This isn't the first time this has happened, either. I've removed quite a few global variables from the various tools over the years. I've run into this issue before with some of the tools. It took me about 25 minutes to figure out the crash the first time it happened tonight. It took about 25 seconds the next time. I have a lot of experience with the debugger, the language and with the bug in particular so I didn't spend days trying to track it down. You may not be so lucky, however. This is a very evil and very subtle bug to have. It can be easily prevented by using as few global variables as absolutely possible. Defining classes that properly accept other dependent objects as parameters and storing references/pointers to them is going to lead to far less headaches down the road than taking the lazy way out and making a commonly used object a global variable. Take the time to do it right from the start and you won't spend hours debugging a weird crash that appears out of nowhere when you were changing unrelated parts of the code.

Share this post


Link to post
Share on other sites
czardas

Geeze, what a pain. I hope you don't encounter anything else like that. I'll stick to using locals in my scripts providing my scripts still work.

Edited by czardas

Share this post


Link to post
Share on other sites
monoceres

Google have a pretty good code rule in their code style guide. Only trivial data types in static and global variables.

I'm also actually taking a class in constructing of programming languages. My basic idea so far is to make a language without a global namespace where even functions need to be declared in local scope. it's going to be interesting.

Edited by monoceres

Broken link? PM me and I'll send you the file!

Share this post


Link to post
Share on other sites
MvGulik
whatever Edited by MvGulik

"Straight_and_Crooked_Thinking" : A "classic guide to ferreting out untruths, half-truths, and other distortions of facts in political and social discussions."
"The Secrets of Quantum Physics" : New and excellent 2 part documentary on Quantum Physics by Jim Al-Khalili. (Dec 2014)

"Believing what you know ain't so" ...

Knock Knock ...
 

Share this post


Link to post
Share on other sites
monoceres

Do you have a link to where they elaborate on that? Just curious.

http://google-styleguide.googlecode.com/...cppguide.xml#Static_and_Global_Variables

As you can see it's just these kinds of things that makes google forbid global/static variables that aren't POD.

Edited by monoceres

Broken link? PM me and I'll send you the file!

Share this post


Link to post
Share on other sites
Negat1ve

Hmm i have problem similiar to this ... but my problem is that i can't create global string variable that can be used by all source files.. Any ideas ? =( i tried everything but it won't work... i m using Managed visual C++ 2008

i tried: extern String^ Variable but it won't work .. please help

Share this post


Link to post
Share on other sites
Valik

So wait, you want to take my thread that gives just one (of many) examples why you shouldn't use global variables and hijack it to ask how to use global variables in Managed C++? Fuck off and be glad I don't ban you for thread hijacking.

Edit: And your problem isn't even related. That goes to show you have no idea what I'm talking about and should never have replied to this thread in the first place. It also shows you don't know C++ and shouldn't be using global variables to begin with since you don't have enough experience or knowledge of the language to know when it's appropriate to use them.

Edited by Valik

Share this post


Link to post
Share on other sites
PartyPooper

Trying to wrap my head around this as I use quite a few global variables in the form of file/window handles in my programs but I'm not having much luck. Is it that on occasions, global variables can be accessed and used before they are actually populated with data due to some rare compiling order problem? I assume the Opt("MustDeclareVars", 1) directive makes no difference to this and that this problem has nothing to do with defining a global variables that's being used by Aut2Exe?

Share this post


Link to post
Share on other sites
Valik

Most use of global variables in AutoIt are trivial. What I'm talking about are C++ objects (classes) with non-trivial constructors. I can easily demonstrate the problem in AutoIt, however, by initializing global variables using functions:

; Pre-declare the variables
Global $g_nFileSize, $g_hFile

; Initialize the variables with data.
$g_nFileSize = GetFileSize()
$g_hFile = FileOpen("thefile.txt", 0)

Func GetFileSize()
    ; Calculate file size using it's position.
    Local Const $nCurrent = FileGetPos($g_hFile)
    FileSetPos($g_hFile, 0, 2)
    Local Const $nRet = FileGetPos($g_hFile)
    FileSetPos($g_hFile, $nCurrent, 0)
    Return $nRet
EndFunc

In that example the variable $g_nFileSize is initialized with the output from GetFileSize(). GetFileSize() uses $g_hFile which hasn't been properly initialized yet. Reversing the initialization of the two variables will solve the problem. However, it may take quite a bit of time to figure out why $g_nFileSize is always zero if this is used in a large script. Syntax checking (Either by Au3Check or AutoIt) cannot help this situation because the variables were pre-declared so they exist. However, it's not quite as likely anyone would write code like this. It would instead be:

; Define the variables.
Global $g_nFileSize = GetFileSize()
Global $g_hFile = FileOpen("thefile.txt", 0)

Func GetFileSize()
    ; Calculate file size using it's position.
    Local Const $nCurrent = FileGetPos($g_hFile)
    FileSetPos($g_hFile, 0, 2)
    Local Const $nRet = FileGetPos($g_hFile)
    FileSetPos($g_hFile, $nCurrent, 0)
    Return $nRet
EndFunc

That code produces a run-time error (but again, Au3Check cannot detect it).

The problem isn't as possible in AutoIt but it's still not hard to do for any non-trivial script. In C++ it's very easy to do when objects that interact are created at the global scope.

  • Like 1

Share this post


Link to post
Share on other sites
PartyPooper

Ahh thanks for the examples - clarifies things somewhat.

Share this post


Link to post
Share on other sites
Jon

http://google-styleguide.googlecode.com/...cppguide.xml#Static_and_Global_Variables

As you can see it's just these kinds of things that makes google forbid global/static variables that aren't POD.

Some interesting stuff in there. Some weird stuff too.

Share this post


Link to post
Share on other sites
monoceres

Some interesting stuff in there. Some weird stuff too.

Anything specific? I like that guide and actually try to think about it when I write code.

The thing I don't agree with is the resistance to RTTI. They also forbid exceptions, something I cannot say anything about since I never use exceptions in C++.


Broken link? PM me and I'll send you the file!

Share this post


Link to post
Share on other sites
James

I quite like this bit from the style guide:

Macro Names▶ You're not really going to define a macro, are you? If you do, they're like this: MY_MACRO_THAT_SCARES_SMALL_CHILDREN.

Edited by JamesBrooks

Share this post


Link to post
Share on other sites
Jon

The ones I found interesting were:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Operator_Overloading#Operator_Overloading

We often have discussions around operator overloading and it was interesting to see such an out and out dislike.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Reference_Arguments#Reference_Arguments

Didn't follow the logic on the second part of this one. It looks like they are saying to use C++ style for inputs and C for outputs. Madness.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Default_Arguments#Default_Arguments

Ouch. I use these quite a bit.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Line_Length#Line_Length

This is not the 80s ffs.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Spaces_vs._Tabs#Spaces_vs._Tabs

Christ. Lots of arguments each way for this. But the one point all these arguments miss is this: using spaces fucks those who prefer tabs up the ass. Once something has been formatted with spaces then a.n.other programmer who prefers more whitespace is simply screwed. The other way around and you can happily adjust tab size or easily convert to spaces later. It absolutely baffles me why Visual Studio switched to spaces with .NET.

And a tab size of 2?? Probably just to fit into the 80 char limit.

Share this post


Link to post
Share on other sites
Valik

The ones I found interesting were:

http://google-styleguide.googlecode.com/...perator_Overloading#Operator_Overloading

We often have discussions around operator overloading and it was interesting to see such an out and out dislike.

It really sounds like some of their policies are meant to fix people issues and not technical ones. What good does it do to make people think about how expensive an operation is, for example, when they are going to do it anyway? So if they are going to do it anyway why shouldn't the object they are working with have named methods when operators would do just as well? Why make programmers remember method names like "Compare" and "Before" when operator==() and operator<() provide sensible, well established behaviors that are consistent between most objects.

http://google-styleguide.googlecode.com/...=Reference_Arguments#Reference_Arguments

Didn't follow the logic on the second part of this one. It looks like they are saying to use C++ style for inputs and C for outputs. Madness.

They lost me at "...as they have value syntax but pointer semantics". Way to miss the point. Of course they have "pointer semantics", that's the entire point. Their whole justification here is about const correctness... which is a separate issue entirely. What does value versus pointer syntax matter if inputs are correctly const and outputs are non-const? Who gives a damn if you type a object.value or object->value?

I can understand their default arguments policy... except as Jon said, this isn't the 80s. IDEs provide help with syntax. There's no excuse "But I didn't know about that optional last parameter".

I wrap my comments to 80 characters, I wrap my code when it makes it easier to read. I probably wouldn't wrap my comments if you damn dirty fixed-width users didn't read it.

http://google-styleguide.googlecode.com/...?showone=Spaces_vs._Tabs#Spaces_vs._Tabs

Christ. Lots of arguments each way for this. But the one point all these arguments miss is this: using spaces fucks those who prefer tabs up the ass. Once something has been formatted with spaces then a.n.other programmer who prefers more whitespace is simply screwed. The other way around and you can happily adjust tab size or easily convert to spaces later. It absolutely baffles me why Visual Studio switched to spaces with .NET.

And a tab size of 2?? Probably just to fit into the 80 char limit.

Jon points out the obvious here. Don't be a dick, use tabs. It's bad enough that I'm probably not reading code formatted how I want to begin with, don't make the indentation all fucked up, too. Spaces waste space and it's easy to fuck up alignment with a typo (You'll quickly spot an alignment issue with tabs). I've never understood how spaces were even a consideration.

Share this post


Link to post
Share on other sites
Bowmore

The ones I found interesting were:

http://google-styleguide.googlecode.com/...uide.xml?showone=Line_Length#Line_Length

This is not the 80s ffs.

That is just mad. It must be a right pain having to stick to that rule. You either end up using very short variable names or statements spread over several lines. Result the code is difficult to read and follow the flow. 

Edited by Bowmore

"Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to build bigger and better idiots. So far, the universe is winning."- Rick Cook

Share this post


Link to post
Share on other sites
Richard Robertson

I don't think that formatting should even matter as long as the code is correct. Good IDEs have formatting rules that you can apply and format the document the way you want it.

That is, when the files are not actively shared between users. For active sharing, the group should pick their own format together.

Edited by Richard Robertson

Share this post


Link to post
Share on other sites
Jon

It really sounds like some of their policies are meant to fix people issues and not technical ones. What good does it do to make people think about how expensive an operation is, for example, when they are going to do it anyway? So if they are going to do it anyway why shouldn't the object they are working with have named methods when operators would do just as well? Why make programmers remember method names like "Compare" and "Before" when operator==() and operator<() provide sensible, well established behaviors that are consistent between most objects.

Most of the C++ books I've looked at say to keep the operation consistant with the operator (like don't use >> to mean "compare") - which is sensible. The argument in the google doc starting talking about operators fooling the user into thinking that operator syntax operations were "cheap". I certainly don't think that a += operation on a string is always cheap. Maybe that's because I spent so long programming the string class to accept that operator. *shrug*

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×