0

Let us assume I have the following C++ class:

class MyFastMessageEncoder
{
public:
    MyFastMessageEncoder() :
        m_fieldEncoder(ENCODING_STYLE_DEFAULT)
    {
    }

    void set_encoding_style(const EncodingStyle encodingStyle)
    {
        m_fieldEncoder.set_encoding_style(encodingStyle);
    }

    EncodingStyle get_encoding_style() const
    {
        return m_fieldEncoder.get_encoding_style();
    }

    void encode(const UnencodedFieldList &unencodedFieldList, Message &message
    {
        for (UnencodedFieldList::const_iterator it = unencodedFieldList.begin(), end = unencodedFieldList.end(); it != end; ++it)
        {
            m_fieldEncoder.set_raw_data(*it);
            message.add_field(m_fieldEncoder.encode());
        }
    }

private:
    FieldEncoder    m_fieldEncoder;
};

The MyFastMessageEncoder class is required to be extremely efficient (encoding will be performed on tens of thousands of messages per second), the time required to instantiate the FieldEncoder object is a non-negligible cost, so to solve the issue, I have defined the FieldEncoder object as a class member, and initialized it inside the constructor just once.

My issue is that FieldEncoder::SetRawData() is a non-const method, which means that the MyFastMessageEncoder::encode method must also be non-const. However, if I were to make the FieldEncoder object a temporary inside the method, MyFastMessageEncoder::encode could be flagged as const. This means that every object that calls MyFastMessageEncoder::encode is also non-const, which (to me) just starts ruining the program's const-correctness.

Now I understand how the mutable keyword works works, and how I could use it to solve this problem, but my question is: Would doing so be considered bad form, since the internals of the object are technically being modified?

Other considerations:

  • Thread-safety is not a concern, this is guaranteed to only run in a single thread (although the MyFastMessageEncoder may be created on multiple threads)
  • The set_raw_data method does not impact the FieldEncoder class outside the MyFastMessageEncoder::encode method. Any data that is set is cleared or overwritten the next time the set_raw_data method is called.
  • 2
    When you're dealing in these kinds of micro-optimizations, doesn't good taste essentially go out the window anyway? – Robert Harvey Jul 23 '14 at 21:53
  • @RobertHarvey Yeah that's pretty much what I'm coming to accept, I'm just lost as to which of the two I'd consider the lesser-of-two-evils :( – Karl Nicoll Jul 23 '14 at 21:55
  • Make the member a function argument. const problem solved. – Thomas Eding Jul 24 '14 at 5:38
1

If encoding depends on previous fields, then MyFastMessageEncoder::encode is obviously non-const as it changes the observable state of the instance - calling it twice won't give the same result, therefore the state of the instance has changed, therefore it should be non-const.

Otherwise it seems a bit odd that you have, in the FieldEncoder, both a costly initialisation and that the state following set_raw_data is governed only by the data that's set. This suggests to me that the initialisation cost might be reduced - either sharing some of whatever structures are set up, or by pooling encoders.

However, both of those refactorings would be much more involved than making the encoder mutable, so if the initialisation cost is not causing problems elsewhere I would comment that the encoder only exists as a member as an optimisation for encode and make it mutable.

  • Hi Pete, thanks for your response! My example is quite simplified to try and get my point across, but I see what you're saying. Unfortunately the FieldEncoder object itself is part of a 3rd party library that I can't change, and their advice is to pre-allocate the encoding object(s). So this question is based more around their advice than my design. The encode method of the MyFastMessageEncoder class does not change the observable state, it simply encodes the data passed to it and packs it, so if the Encoder were declared as a temp, it would definitely be const. – Karl Nicoll Jul 23 '14 at 22:49
  • Your thought about pooling is interesting though, it might just work! – Karl Nicoll Jul 23 '14 at 22:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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