Is this too non-mutable to const?

Question

Basically, I have the following situation. Note: void* is used to denote arbitrary data, it is strongly typed in a real application.

class A
{
public:
   //uses intermediate buffer but doesn't change outward behavior
   //intermediate buffer is expensive to fill
   void foo(void* input_data);

   //uses intermediate buffer and DOES explicitly change something internally
   //intermediate buffer is expensive to fill
   void bar(void* input_data);

   //if input_data hasn't changed since foo, we can totally reuse what happened in foo
   //I cannot check if the data is equal quickly, so I allow the user to pass in the
   //assertion (put the honerous on them, explicitly tell them in the comments
   //that this is dangerous to do, ect)
   void bar(void* input_data, bool reuse_intermediate);
private:
   void* intermediate_buffer_;
   void* something_;
};

So trying for const correctness, intermediate_buffer_ is never exposed so it sortof fits the definition for using a mutable variable. If i never reused this buffer, or I checked for equal input_data before using the cached values, that would be the end of the story, but because of the reuse_intermediate I feel like I'm half exposing it, so I'm not sure whether or not the following makes sense.

class A
{
public:
   //uses intermediate buffer but doesn't change something
   //intermediate buffer is expensive to fill
   void foo(void* input_data) const;

   //uses intermediate buffer and DOES explicitly change something internally
   //intermediate buffer is expensive to fill
   void bar(void* input_data);

   //if input_data hasn't changed since foo, we can totally reuse what happened in foo
   //I cannot check if the data is equal quickly, so I allow the user to pass in the
   //assertion (put the honerous on them, explicitly tell them in the comments
   //that this is dangerous to do, ect)
   void bar(void* input_data, bool reuse_intermediate);

   //an example of where this would save a bunch of computation, though
   //cases outside the class can also happen
   void foobar(void* input_data)
   {
      foo(input_data);
      bar(input_data,true);
   }
private:
   mutable void* intermediate_buffer_;
   void* something_;
};

Thoughts?

Answer 1

I think this is improper use of mutable. In my experience mutable is used for ancillary private member variables that by their very nature can not be declared const, but do not modify the "conceptual constness" of the public interface.

Take for example a Mutex member variable and a 'thread-safe getter':

class Getter { 
public: 

    Getter( int d, Mutex & m ) : guard_( m ), data_( d ) { };

    int get( ) const { Lock l(guard_); return data_; };

private:

    mutable Mutex guard_;
    const int data_;
};

The main point here is that the data declared mutable (in this case the guard) does change (it's locked and unlocked) but this has no impact on constness from the users perspective. Ultimately, despite the mutable Mutex, you still can't change the const data_ member variable and the compiler enforces this.

In your case, you really want the intermediate_buffer to be const but you explicitly tell the compiler it's not by declaring it mutable. The result is that you can change the data and the compiler can't do a thing about it.

See the difference?

If you really want the interface to live up to the const agreement, make it explicit via something like this:

    class A { 
    public:    

        A( void* input_data );// I assume this deep copies.

        void foo() const;

        void bar();

        private:    
            const void* intermediate_buffer_;   
            void* something_; 
    };

Now the onus is truly on the user and enforced by the compiler regardless of what the comments say and without any use of mutable. If they know that input_data has changed, they'll have to create a new one, preferably const.

Answer 2

Instead of hiding the intermediate object inside the const object, expose it and let foo and bar return a copy. You are making it very explicit that the intermediate object is being shared, and providing a new capability to keep more than one on hand. If you want to hide the implementation details you can expose an empty class and make the intermediate object a child of this base class.

class A
{
public:
   class Intermediate
   {
      //empty
   };

   //uses intermediate buffer but doesn't change outward behavior
   //intermediate buffer is expensive to fill
   //if cache_ptr is NULL the intermediate buffer is discarded
   void foo(void* input_data, Intermediate** cache_ptr = NULL) const;

   //uses intermediate buffer and DOES explicitly change something internally
   //intermediate buffer is expensive to fill
   void bar(void* input_data, Intermediate** cache_ptr = NULL);
private:
   class IntermediateImpl : public Intermediate
   {
      //...
   };
   void* something_;
};

Answer 3

To answer your question directly. If the function foo is const, calling it at any time should never alter the result of the next operations.

for example:

A a(some_big_data);
a.bar(some_big_data, true);

should give exactly the same result as (excluding performance differences)

A a(some_big_data);
a.foo(some_different_big_data);
a.bar(some_big_data, true);

Since foo is const, users will expect the result to be the same. If this is the case then making the buffer mutable is reasonable. Otherwise, it is probably wrong.

Hope this helps