Bad practice or acceptable?

Say you have a singleton thats accessed like this ....

SomeSingletonClass::Instance()->FunctionCall();

would doing ....

#define gSomeSing SomeSingletonClass::Instance()

and then making calls like this ...

gSomeSing->FunctionCall();

be a dumb thing to do ? Seems to work, but are there any down falls not related to naming?

Macro's are a touchy subject. I don't see anything wrong with doing that. Just remember that you did that while you're debugging because it tend to cause confusing errors. It's easy to create a bug in a macro and never find it because you forgot you even made the macro.

Also, if you use that to call several functions continuously, it might benefit you to hold the result of Instance() in a seperate pointer.

Right, as skorman00 was implying, the following is a much better idea:

Code:

---

SomeSingletonClass* inst = SomeSingletonClass::Instance();
inst->whatever(); // ... etc ...

---

Remember that macros actually change your code before the compiler sees it, so the compiler does not see the same thing that you do. In this case, there is probably little harm, but in general, I'd recommend not using macros unless there is a truly compelling reason to use them.

I think that's bad practice because it disguises the use of the singleton. It's not dumb though. I just think having two names for a singleton object a bit confusing. You'd also need to develop a standard naming convention for your #define's to avoid this problem.

Yes you are right, I can see if your making several function calls with this singleton within a single function it would be best to store the pointer at the top of the function and call however many times. Plus saving the overhead if the call is something like this

Code:

---

ManagedObject* mobj = SomeSingletonObjct::Instance()->GetManagedObject("name");

---

where GetManagedObject() may have to do a search, writing

Code:

---

mobj->DoSomething();

---

and searching once is much better than writing this

Code:

---

SomeSingletonObjct::Instance()->GetManagedObject("name")->DoSomething();

---

and searching eight times. What I originally was looking to do was to quckly shorten something like this

Code:

---

SomeSingletonObjct::Instance()->GetManagedObject("name")->DoSomething();

to

gSomeSing->GetManagedObject("name")->DoSomething();

or

gSomeSing->DoSomething();

---

when its being used in a place where it only needs to be called once and storing the pointer has no purpose because its used once either way. Thx for the replies.

Perhaps better than a macro is an inline function that returns the instance:

Code:

---

inline SomeSingletonObjct* gSomeSing() { return SomeSingletonObjct::Instance(); }
// or
inline SomeManagedObject* gSomeSing() { return SomeSingletonObjct::Instance()->GetManagedObject("name"); }

// use as
gSomeSing()->DoSomething();

---

I'd also return a reference to the singleton instead of a pointer, but that's of course a completely different subject.