0

This was just a question I was wondering about in terms of best practice. I am writing a small C++ program that uses an interface to implement a function.

In the class that uses the interface (i.e. inherits from the interface class), is it bad practice for me to use member variables so I don't have to keep re-declaring them every time the function comes into scope?

Say we have this interface:

class base{
public:
    virtual ~base() {};
    virtual int method(int num) = 0;
};

And the following class inherits it to implement the method function.

class usesinterface : public base {
public:

    usesinterface() {};

    virtual int method(int num){ // Always adds 5 to the number
        return num_ + num; 
    }

private:
    int num_ = 5;
};

Is storing num_ as a class member bad practice or considered wrong? Does it make more sense to write:

virtual int method(int num){
    const int number = 5;
    return number + num;
}

I am aware you can just write + 5 to save the hassle for this example but this is just a simplified version of a problem I am looking at.

EDIT 1:

In reality, the variable I am storing in my class / re-defining every function call is a lookup table that is only used within this singular function call. However, if I put it in the function, the function itself becomes very verbose (much worse code readability) and without optimization I believe it would continually re-allocate the lookup table.

1
  • 1
    If the initialisation is one statement, you can make it a static local, and the initialisation will run once. The constness of your example suggest it is
    – Caleth
    Jul 30, 2018 at 8:39

3 Answers 3

2

The problem is that it is very situation dependent. In the sketched scenario, the local variable is definitely preferred; there's no reason to have class scope for it.

However, if _num is the result of a complex calculation of which the result can be stored for later re-usage then it's an obvious choice to store it in a member variable, rather than recomputing it all.

Generally one should prefer whatever has the smallest needed scope, especially for constants. If the situation allows you to avoid a bunch of calculations, there's no reason to not use a member.

Edit: based on your comment of it being something like a large lookup table, you can always reduce the cruft by separating that lookup table in a separate function, isolating the lookup table. I.e.:

static int lookupNum(int index)
{
    // Lookup table stuff
    return lookup_table[index];
}

int Foo::doStuff()
{
    // do some calculations
    return lookupNum(index);
}

Regardless, you will have to place the lookup table somewhere. Making it a member does not make it more readable. You've simply moved the verbosity to somewhere else, where it's less obvious that it is only used by one function.

2
  • I added to my question to explain my situation in a bit more detail
    – WoodyDev
    Jul 30, 2018 at 8:15
  • Was already editing the answer when you posted your comment. I think my edit answers more precisely what you are asking for
    – Athos vk
    Jul 30, 2018 at 9:48
0

The answer is within Object Oriented Programming principles. If num_ belongs to the usesinterface class, it must be defined as a class variable. If it doesn't belong to usesinterface class and is used for temporary information processing then it should be scoped within a method body.

In another way, if num_ defines state of the object then it should be defined as a class variable. There is one exception though. In some complex application, it is possible that you declare a class variable just to share the value of the class variable across private or public method calls. In such case, developers use class variables. Still if you look at it, these class variables form state of the object.

2
  • Hi! In my case, num_ will only be used within this one function call within usesinterface class. However, the variable inside the function makes the function considerably more verbose (in reality I have a ~15 element lookup table) and requires the lookup table to be re allocated every function call - unless the compiler optimizes this out. So in reality it doesn't need to be a class member, but for code readability it should? This is where I'm not sure
    – WoodyDev
    Jul 30, 2018 at 8:10
  • In such a case, if it really helps in limiting the reallocation of the lookup table and improve readability, you can use num_ as a class member as an exceptional case. You can optimize it later keeping it as a technical debt.
    – sshah
    Jul 30, 2018 at 8:17
0

Don't use persistent state unless you need it to persist.
Reasons you might need it include:

  • Recreating it is too expensive, so caching it is needed to meet performance-requirements.
  • It is dynamically modifiable, and letting the caller pass it in is inappropriate. No way around it then.

Neither of those seem to apply here, so don't.
A constexpr (guaranteed evaluated at compile-time) might be appropriate though, or at least a static const (only evaluate once, possibly at compile-time, otherwise at first encounter if in a function or on startup if not).

As an aside, when reading your title I thought you were trying to decide on using free-functions or member-functions for your interface, instead of how to inherit from your abstract base-class. Also, you said "use" but meant "inherit" or at least "implement"; use is normally on the side of the consumer instead of implementer.

Finally, your ABC should be neither copyable nor moveable, though descendants might be. So you need to add:

protected:
    base(base const&) = default;
    base& operator=(base const&) = default;

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.