Think twice, cut once

Sometimes it seems to me that tools are getting too important. Don’t get me wrong, I really appreciate those little helpers modern IDE’s and their plugins are offering. For example if they’re suggesting to me a method might be rather static instead of const. Or not all paths of a lengthy, chained and nested If statement are returning something. Especially with legacy code those tools are very helpful.

But sometimes I stumble upon code where you can immediately see that the tools quick offered help was easier to apply then thinking twice about a problem. I would go so far to say that this behavior shows up for every developer sometimes, especially under pressure. “This feature has to be finished till the end of the week, we promised the customer.” “But the sources are barely beyond prototyping state and not cleaned up at all. Well ok I let the tools do their work, so it’s at least in a certain cleaned up state.” Sounds familiar? If I’m honest, to me it does. Is it bad? Well, the tools do their job, they do what their made for and they do nowadays a really good job.

But still, trusting them blindly can, sooner or later, lead you into problems. And the price to pay might be higher than you think upfront. For example, I’ve seen code where the tool exchanged types with auto as much as possible. At the end it was not a problem with buggy code, it was more a problem of readability. The code was now partly much harder to understand because the types became obfuscated. That wasn’t the intention of introducing auto into C++, for sure not.

A more subtle, but in my opinion much more dangerous, problem occurs with transforming methods into const methods blindly, only because a tool is suggesting it. A const method is not really changing the binary code a compiler is producing (if I’m wrong with my assessment feel free to correct me). It’s more to promise to the developer, and his companion and/or users of his library, that a call of a particular method won’t change the state of a class instance. If a method foo() of a class Bar is const, I could call it as many times I want, it wouldn’t change the state of an instance of Bar.

But let’s have a look at this little and simple code snippet:

A class with an array as member. We will just focus on the const, nothing of all the other problems MyClass has. That class was “optimized” by a tool, and it’s correct from the compilers point of view, no errors, no warnings, awesome. If we would just see the interface, or let’s say the contract, MyClass is offering, we would think that getArray is harmless and has no side effects. But unfortunately getArray has a side effect we would probably realize to late. The method is incrementing the first element of the array every time getArray will be called. How can that be if this method is const? There where no compiler errors nor warnings. And in principle it’s correct. The pointer to the first array element is never changing, but it’s content is. But this means the objects state has changed, does it? Well, if you see only the pointers, the objects state didn’t change. If you know more about the implementation you will realize it’s not true what getArray is promising, not changing the objects state.

This way the tool is sabotaging our encapsulation we want to accomplish and preserve with our interfaces and contracts. We would need to know the internal implementation of MyClass to use it correctly. Thinking twice, before we trusting a tool blindly and cut the rope which is preventing us from falling into the abyss, is, in my opinion, one of the more important skills, every engineer should practice.

Did you like this post?

What are your thoughts on this post?

Feel free to comment and share the post.

Processing…
Success! You're on the list.

11 thoughts on “Think twice, cut once

  1. I would like to point out that const has a double meaning. 1) a const method promises not to change the OBSERVABLE state of the object; internal state may be changed, and that’s where the keyword mutable comes in. 2) a const method may be called on a const instance of the object, a non const method may not. so this code will not compile:

    const MyClass c;
    c.setFirstVal(1);

    if the content of the array affects the observable state of the object then getArray violates #1.

    Like

    1. Yes I agreed. I’m a bit ambivalent concerning mutable. I know of it’s benefits and drawbacks, but it’s banned in our company for certain reasons. Always stick to the company coding guidelines and adapt them if necessary😉

      Like

      1. I can see why a coding guideline would ban it for junior developers, but it is a very powerful and useful tool. for example, you can have a mutable method call counter on an object that can count invocations of const and non-const methods; or do other type of book keeping and statistic data collection in the background, invisible to the observable state of the object. i don’t see drawbacks to it other than it can be misused by an inexperienced developer who wants to quickly fix a compile error, so he/she marks a variable mutable, and instantly violates the contract of the class’s interface 🙂

        Like

  2. Here’s a complete illustration of both meanings of const:


    class MyClass {
    public:
    // const method, changes internal state
    void constMethod() const { m_nonVisibleState = 1; }
    // non const method, changes observable state
    void setState(int s ) { m_visibleState = s; }
    // const method, does not change observable state
    int getState() const { return m_visibleState; }

    private:
    mutable int m_nonVisibleState = 0;
    int m_visibleState = 0;
    };

    int main()
    {
    MyClass c1;
    c1.constMethod(); // ok
    c1.setState(1); // ok
    c1.getState(); // ok

    const MyClass c2;
    c2.constMethod(); // ok
    c2.setState(1); // error
    c2.getState(); // ok
    }

    Like

    1. second attempt at code formatting in the comments…

      class MyClass {
      public:
      	void constMethod() const { m_nonVisibleState = 1; }
      	void setState(int s ) { m_visibleState = s; }
      	int getState() const { return m_visibleState; }
      	
      private:
      	mutable int m_nonVisibleState = 0;
      	int m_visibleState = 0;
      };
      
      int main()
      {
      	MyClass c1;
      	c1.constMethod(); // ok
      	c1.setState(1); // ok
      	c1.getState(); // ok
      
      	const MyClass c2;
      	c2.constMethod(); // ok
      	c2.setState(1); // error
      	c2.getState(); // ok
      }
      

      Like

    2. third attempt at code formatting…

      class MyClass {
      public:
      	void constMethod() const { m_nonVisibleState = 1; }
      	void setState(int s ) { m_visibleState = s; }
      	int getState() const { return m_visibleState; }
      	
      private:
      	mutable int m_nonVisibleState = 0;
      	int m_visibleState = 0;
      };
      
      int main()
      {
      	MyClass c1;
      	c1.constMethod(); // ok
      	c1.setState(1); // ok
      	c1.getState(); // ok
      
      	const MyClass c2;
      	c2.constMethod(); // ok
      	c2.setState(1); // error
      	c2.getState(); // ok
      }
      

      Like

Leave a Reply to bmahr Cancel reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.