tags:

views:

386

answers:

7

Say I have:

class A {
public:
    static void DoStuff();

    // ... more methods here ...
};

And later on I have a function that wants to call DoStuff:

B::SomeFunction(A* a_ptr) {

Is it better to say:

    a_ptr->DoStuff();
}

Or is the following better even though I have an instance pointer:

    A::DoStuff()
}

This is purely a matter of style, but I'd like to get some informed opinions before I make a decision.

+21  A: 

I think I'd prefer "A::DoStuff()", as it's more clear that a static method is being called.

Fred Larson
Though I really like Adam's example, I'll give the check for this subjective question to the crowd favorite.
jeffamaphone
+6  A: 

I personally prefer the A::DoStuff() convention because it's immediately clear to anyone reading the code that it's a call to a static member function.

Ben Collins
+18  A: 

It's better to call the static method by its name, not through an object, since it doesn't actually use that object at all. In Java, the same problem exists. A not-too-uncommon problem in Java is the following:

Thread t = getSomeOtherThread();
t.sleep(1000);

This compiles fine but is almost always an error -- Thread.sleep() is a static method that causes the current thread to sleep, not the thread being acted on as the code seems to imply.

Adam Rosenfield
Good point. I suppose similar errors could arise in C++ frameworks.
jeffamaphone
If the API does have this, then I think this is a major "bug" in the API. Maybe it's because Java doesn't have the concept of free functions that it had to do this - but that isn't necessary in C++. A function should be a member function if it has special access to the object (eg. private constructor).
Richard Corden
+2  A: 

Although I agree that A::DoStuff() is clearer, and it's what I'd write myself, I can see an argument for going via the pointer, which is "suppose the class name changes". If class A becomes class B, then we only need to update the class name in one place (the pointer declaration) instead of two.

Just a thought...

anon
Also, the static method might someday change to an instance method; I've seen that happen on occasion.
Dan Breslau
Either change will cause compile time errors. Changing names is a simple matter of search and replace (usually--works fine with names more descriptive than "A" and "B").
jeffamaphone
A: 

Generally I do the A::DoStuff(); way instead of a->DoStuff(); because maybe someday the function I'm in won't have that instance pointer anymore due to refactoring. But it's a total style thing that you shouldn't loose any sleep over.

Brian Neal
+1  A: 

Jon Skeet opened my eyes to why you should not call a static method through an instance pointer. His example is in Java, but the concept applies to C++, too:

Thread t = new Thread(...);
t.start();
t.sleep(1000); // Which thread does it look like this will affect?

As I commented when I first read his answer: "Until I read [Jon's post], I considered being able to call static methods through an instance reference a feature. Now I know better."

In short, call static methods using the class name, not an instance. In my opinion, it's more than a style issue - it can result in misleading, buggy code.

Michael Burr
This looks like a really contrived example. If you have a sleep member in a Thread class then if that member does *not* affect just that thread then you should shoot the person who wrote the API.
Richard Corden
Another answer mentions this too - so this must be an issue in Java. However, a flaw in the design of a Java API (because of the lack of free functions) is not necessarily a good argument for something in C++.
Richard Corden
While the example given was actually Java, it is more-or-less valid C++ as well, given a Thread class with a start() instance method and a static sleep() method. Of course, you'd also either have to get rid of the `new` or make `t` a pointer and change the `t.start()` to `t->start()`, etc. - but those changes are besides the point about calling a static method through an instance.
Michael Burr
Also, you may want to get out your gun - a Thread class with a static sleep() method seems to be quite common. Even boost::thread has it, and while the boost developers are not perfect, I'd be reluctant to call them worthy of shooting.
Michael Burr
A: 

I've seen many questions in Java where if people called a static method using the syntax of calling an instance method through an object, and the object is actually a subclass of the variable type, people wonder why it doesn't call a static method of the same name in the subclass. The fact that they are calling it through an object makes them think that it is an instance method which can be overridden and it somehow does runtime dynamic lookup using the type of the object.

But of course the object is never used in the call at all -- only the type of the variable is used at compile-time to decide what class's static method it is. So if you put an object there, it is completely misleading because it makes people think that it is used, when it is not. That's why I favor only calling static methods through the class name -- it is exactly equivalent to calling it through a variable of the class type; but it tells you exactly what is going on, with no useless misleading extra information.

newacct