To static or not to static?

JonB

Often we define private "helper" methods in a class to do bits of calculation which do not access any class member variables or methods, like:

class Foo
{
private:
    int helper(int a, int b, int c) const { return a * b * c + a - b + c; }
};

From my recollection of C# it warned if you had a non-static member method which does not access this, suggesting you make it static. Which I got into the habit of doing.

Do the C++ experts regard this as good/bad/indifferent practice for C++ (where gcc at least does not give such a warning)? I am tending to define them as static too.

J.Hilk

@JonB I didn't expect an answer to that "question" it was rhetorical and supposed to be the answer.

A: "I wouldn't bother with making private functions static, IF they aren't called by public static functions"

J.Hilk

@JonB Both approaches have their place, and the choice depends on the context and design goals of your class.

Static:
Pros:

No need for an instance to call the method.
Lower memory overhead (no this pointer).
Can be called using the class name.

Cons:

Cannot access non-static members.
Cannot be overridden in derived classes.
Not part of polymorphism.

Non-Static
Pros:

Can access other non-static members.
Can be overridden in derived classes.
Can be used in polymorphism.

Cons:

Slightly higher memory overhead due to the hidden this pointer.
Requires an instance of the class to call the method.

JonB

@J-Hilk
Thank you, I know the pros & cons. I know both are possible. I know how each work. Clearly overriding, polymorphism, accessing other non-static members are not of interest here. Your comments are really about practical details. My question is which you would choose to use, let's take the case of the method I gave as example?

J.Hilk

@JonB in that particular case, why even think about making it static, when it's a private function?

JonB

@J-Hilk said in To static or not to static?:

about make it static, when it's a private function?

I answered that, by saying "C# warns in such a case (no accessing of this in a method) and suggests you change to static, which I do", and asking what C++ experts feel about doing so in C++, even if the compiler does not warn (and I know why it does not)? Which is the reason I am asking the question.

J.Hilk

@JonB I didn't expect an answer to that "question" it was rhetorical and supposed to be the answer.

A: "I wouldn't bother with making private functions static, IF they aren't called by public static functions"

JonB

@J-Hilk said in To static or not to static?:

A: "I wouldn't bother with making private functions static, IF they aren't called by public static functions"

Fair enough!

TomZ

@JonB static methods on a class should, in my opinion, be very strongly related to the class.
Which means things like factory methods and things like QObject::connect

So, no, I'd not make it a static method since it doesn't make sense to do so.

In C++ the thing to worry about is exportable symbols. The ideal is that you have a method that does not leave the cpp file and there static, or simply placing it in an anonymous namespace make sense. It avoids its symbols being seen by the linker.

If you need it in multiple cpp files, I tend to make a private class or namespace where I place them on. Namespace is nice for "static" like methods here. Avoid installing the header file (I follow Qts example of having _p.h as extension) to make it as private as you can.

I personally try to avoid "polluting" a header file with methods that are pure implementation specific. The example qualifies as its pretty cheap to just hide it from the outside world in the ways I suggested.

JonB

@TomZ
Two votes out of two responses for not doing static on a (private) method which does not access this. Shame for not being as "tidy" as I like it in C#, but if that's the C++ convention I will change over to it (obviously marking them as const instead)....

Will await any further opinions and mark this as "Solved" later.

J.Hilk

@JonB const, noexcept and [[nodiscard]] where ever possible ! :D

JonB

@J-Hilk If you think I'm adding that sort of stuff to all my functions I'm not going to! :) I don't think even Qt headers do the "noexcept and [[nodiscard]] " mostly.

J.Hilk

@JonB this actually would be good practice :P

JonB

@J-Hilk It would, but it's too much typing every time to change the defaults. If I wanted that I'd consider changing to Rust ;-)

J.Hilk

@JonB to bad that people refuse to change the standard or at least to add a flag to the compiler to change the default interpretations.

Immagine people having to write [[maybe_unused]] or mutable, the horror!

mranger90

For what its worth, the coding guidelines for my company say that such methods should be external to the class. We tend to bundle them up in common libraries and wrap them in their own namespace.

JonB

@mranger90
FWIW, personally I find that an extraordinarily burdensome guideline. Small utility methods which happen not to use this but are useful privately in a class (and perhaps meaningless elsewhere) are not at all uncommon. (I haven't done so, but I would be surprised if I don't find such in, say, Qt sources.) Having to move them out does not seem nice to me. But what do I know, I respect you say that is what your company requires.

EDIT
I don't have downloaded Qt sources. Just using woboq for Qt5 and having a look at, say, QString for private static methods I happenstanced on, say, declaration https://codebrowser.dev/qt5/qtbase/src/corelib/text/qstring.h.html#_ZN7QString14compare_helperEPK5QChariS2_iN2Qt15CaseSensitivityE

private:
    static int compare_helper(const QChar *data1, int length1,
                              const QChar *data2, int length2,
                              Qt::CaseSensitivity cs = Qt::CaseSensitive) noexcept;

definition https://codebrowser.dev/qt5/qtbase/src/corelib/text/qstring.cpp.html#_ZN7QString14compare_helperEPK5QChariS2_iN2Qt15CaseSensitivityE

/*!
    \internal
    \since 4.5
*/
int QString::compare_helper(const QChar *data1, int length1, const QChar *data2, int length2,
                            Qt::CaseSensitivity cs) noexcept
{
    Q_ASSERT(length1 >= 0);
    Q_ASSERT(length2 >= 0);
    Q_ASSERT(data1 || length1 == 0);
    Q_ASSERT(data2 || length2 == 0);
    return qt_compare_strings(QStringView(data1, length1), QStringView(data2, length2), cs);
}

A perfect example of a private utility method that they seem to declare static as I would have done (though I have not checked whether it might be called from some static method).

JoeCFD

@JonB said in To static or not to static?:

compare_helper

qt_compare_strings is static as well. That may be the reason why compare_helper is static. It does have to be, I guess. If not, the return is different.
return QString::qt_compare_strings(QStringView(data1, length1), QStringView(data2, length2), cs);

If the static function performs operations that are related to the class as a whole, rather than specific instances, it may make sense to keep it static and private.

I do not know if it is a good practice or not. all math funcs for exampes std::cos are defined in namingspace std. The reason could be that template for class member funcs were introduced in C++ 14. In or before C++ 11, it was not possible to use template for member funcs in a class.

JonB

@JoeCFD said in To static or not to static?:

qt_compare_strings is static as well. That may be the reason why compare_helper is static

Sorry, that does not make sense. You can of course call a static function from a non-static member function, just not the other way round. This would not be any reason to choose to make the calling method, QString::compare_helper(), be static.

it may make sense to keep it static and private

But you can see the other responders here are suggesting I do not make private methods static, which I am now following.

JoeCFD

@JonB I guess both ways work. I do not have preference.