-2

Yesterday I was trying to make my globals look better and I came up with this.

// Event.hpp

namespace GEngine{
    namespace Event{
        struct _Mouse{
            int mouse_x;
            int mouse_y;
        };
        extern GEngine::Event::_Mouse Mouse; // Inside
    }
}

// Main.cpp

// Set

GEngine::Event::Mouse.mouse_x = 100;
GEngine::Event::Mouse.mouse_y = 50;

// Get

int foo = GEngine::Event::Mouse.mouse_x;

---- vs -----

namespace GEngine{
    namespace Event{
        struct _Mouse{
            int mouse_x;
            int mouse_y;
        };
    }
}

extern GEngine::Event::_Mouse Mouse; // Outside

.....

// Main.cpp

// Get

Mouse.mouse_x = 100;
Mouse.mouse_y = 50;

// Set

int foo = Mouse.mouse_x;

Which coding style would you choose? Clarity over simplicity or the other way around?

And to be a little bit more accurate. Would you choose to name your global variable GEngine::Event::Mouse.mouse_x or Mouse.mouse_x ?

NT_SYSTEM
  • 367
  • 1
  • 6
  • 14
  • Hard to tell as the formatting of the question isn't entirely correct. – Almo May 10 '12 at 21:20
  • 1
    Try [CodeReview.SE](http://codereview.stackexchange.com). – ildjarn May 10 '12 at 21:21
  • 1
    I think the first is the least evil. It reduces the possibility of name clashes and at least gives a moderate clue what "Mouse" is. Still hate it though. – Edward Strange May 10 '12 at 21:21
  • @Crazy Eddie That's exactly what I was thinking. – NT_SYSTEM May 10 '12 at 21:23
  • 3
    *Aside*: Your program violates the standard by using a reserved name: `_Mouse`. http://stackoverflow.com/a/228797/8747 – Robᵩ May 10 '12 at 21:27
  • Not much point in having the namespaces if you're not going to use them. Once I put *any* symbol in `::`, I'd be tempted to put *all* of them there. – Robᵩ May 10 '12 at 21:28

1 Answers1

0

The short answer.

Neither solution shows good C++ code style. Although to answer the question, the members you are accessing should be fully qualified by the same namespace hierarchy as they are stored.

The long answer.

  • Globals should be constants.
  • If a private member with accessor methods will work, then that should be used instead.
  • Globals are a bad idea in most situations.
  • The accepted method for exposing global constants is not represented in either of your solutions.

A couple of good guides on style

Zero
  • 1,147
  • 1
  • 9
  • 13
  • 1
    Google style guide is debatable, to the very least. It is meant to be used in an environment which is not very C++ friendly to start with. Don't use it. – Alexandre C. May 10 '12 at 21:36
  • Ive found it to contain many of the best industry practices I have seen. While certainly not every item in the guide applies globally to all projects, I would say at the very least, the majority of the guide is extremely useful. – Zero May 10 '12 at 21:39
  • Very debateable -imho- using globals saves time and It's pretty much the same with passing them in structs as function arguements, if used with caution.Also they make the code look lighter. – NT_SYSTEM May 10 '12 at 21:43
  • I would be curious to see specifically which practices in the Google guide you disagree with. – Zero May 10 '12 at 21:44
  • "Globals are a bad idea in most situations". This. – NT_SYSTEM May 10 '12 at 21:45
  • @Zero: I don't like `All parameters passed by reference must be labeled const. ` – Jesse Good May 10 '12 at 21:49
  • Let me clarify, globals are bad in most situations because they violate objected oriented design, meaning the variable itself is more easily understood when its scope is limited as much as possible. Futhermore, there is no access control or constraint checking, they pollute the namespace, and create a tight coupling rather than an implicit coupling of data. The only time I can imagine using a global, non-constant variable is in a one-off program that you would never show to a customer. – Zero May 10 '12 at 21:52
  • In 20 years of software development, and millions of lines of code written, I have still yet to come across a reason to expose a non-constant global variable. While I am sure there exists some situation where it may be prudent, I still hold by my statement that in "Most" situations its simply terrible code style born from lazy development practices. – Zero May 10 '12 at 21:55
  • 2
    I particularly don't like "We do not use C++ exceptions". The main reason they put is that they have existing code which plays bad with them. Fair enough, but this is plain stupid if you don't have that strong a requirement.. – Alexandre C. May 10 '12 at 22:02
  • Exceptions cause the stack to unwind before being handled which is a substantial performance hit compared to other error handling mechanisms. In any high performance system exceptions are frowned upon for that very reason. In many cases exceptions are the simplest and most maintainable way of handling an error, which is why many Apis offer both exception and error code based solutions – Zero May 10 '12 at 22:28
  • Googles guide is only one of many that make that same recommendation. Even the c++ library implementations avoid exceptions wherever possible to maximize performance. – Zero May 10 '12 at 22:31
  • As far as the const parameter recommendation, that refers to incoming parameters only, in general functions should not side effect its parameters for a lot of good reasons. Furthermore, having const parameters allows the compiler to optimize code far more effectively due to assumptions it can make as a result. Again that's not just Google's recommendation, but rather a widely recommended practice in industry. – Zero May 10 '12 at 22:36
  • @Zero: Actually I'm pretty sure its for backwards compatibility with C, because it definitely hinders good idiomatic C++. – Jesse Good May 10 '12 at 23:32
  • I'm just repeating text book data from my compiler design class. Const parameters are just one of many ways to drastically improve the quality of the compiler generated binary. Backwards compatability with C has nothing to do with it. – Zero May 10 '12 at 23:37
  • @Zero: I think we are talking about two different things, Google has a huge C code base and have to make sure code is compatible (its the same thing with exceptions as Alexandre C mentions) – Jesse Good May 10 '12 at 23:42