8

I have two versions of the same static member function: one takes a pointer-to-const parameter and that takes a pointer-to-non-const parameter. I want to avoid code duplication.
After reading some stack overflow questions (these were all about non-static member functions though) I came up with this:

class C {
private:
  static const type* func(const type* x) {
    //long code
  }

  static type* func(type* x) {
      return const_cast<type*>(func(static_cast<const type*>(x)));
  }
public:
  //some code that uses these functions
};

(I know juggling with pointers is generally a bad idea, but I'm implementing a data structure.)

I found some code in libstdc++ that looks like this:
NOTE: these are not member functions

static type* local_func(type* x)
{
  //long code
}

type* func(type* x)
{
  return local_func(x);
}

const type* func(const type* x)
{
  return local_func(const_cast<type*>(x));
}

In the first approach the code is in a function that takes a pointer-to-const parameter.
In the second approach the code is in a function that takes a pointer-to-non-const parameter.
Which approach should generally be used? Are both correct?

Xack
  • 101
  • 1
  • 3
  • 6
    get the terminology right. Static class members **can not** be const. – SergeyA Jan 04 '16 at 17:58
  • You could also use templates to avoid code duplication, though that may be overkill if `type` is hardcoded. – Manik Jan 04 '16 at 18:02
  • 2
    @SergeyA I wrote "(by this I mean they take and return pointers to const/non-const objects)" – Xack Jan 04 '16 at 18:03
  • 1
    @Xack, and you should not do this. `const function` is a term which has it's meaning, if one starts to add different meanings to the widely understood terms, we would loose any possibility to communicate. – SergeyA Jan 04 '16 at 18:06
  • @SergeyA You're right. I'll edit this. – Xack Jan 04 '16 at 18:09
  • 1
    I don't get this. Why have two functions at all? If the function modifies something, then it should take a non-const pointer. And if the caller wants to modify a const object, it's their job to cast. If the function doesn't modify anything, then it should accept const. – Sergei Tachenov Jan 04 '16 at 18:14
  • @SergeyTachenov the function doesn't modify anything, it just returns a different pointer. – Xack Jan 04 '16 at 18:17
  • 3
    If it doesn't modify anything, it should accept a const pointer and return a non-const pointer. I still don't get why there are two functions. – Sergei Tachenov Jan 04 '16 at 18:21
  • @SergeyTachenov It seems to me you're right. (It's a tree structure, `type` is just a node, so it contains pointers to other nodes. The function just travels through the pointers to a different node. But `const` doesn't propagate through pointers.) I just saw the libstdc++ code and assumed I must do the same. – Xack Jan 04 '16 at 18:34
  • I'll try to put it in the form of answer then. – Sergei Tachenov Jan 04 '16 at 18:36
  • Possible duplicate of [How do I remove code duplication between similar const and non-const member functions?](http://stackoverflow.com/questions/123758/how-do-i-remove-code-duplication-between-similar-const-and-non-const-member-func) – mpromonet Jan 04 '16 at 20:26
  • @mpromonet: That's about the non-static member functions, as the question mentioned. But you're quite correct that all of those techniques (including typesafe template versions) will work just fine in the static case as well. – Ben Voigt Jan 04 '16 at 20:39
  • Consider that when there is a pointer implied in a type expression, the "const" differs. Are you working with "const *sometype" or "const sometype", that changes the result of your question, a lot. – umlcat Jan 04 '16 at 21:13
  • @SergeyTachenov You can't get a non-const pointer from a const object, that would break const correctness. This example is no different fundamentally from ``vector::operator[]``, which has const and non-const overloads just as the OP describes. – Nir Friedman Jan 04 '16 at 21:30
  • @Nir, where did I say that you _can_ get a non-const pointer? Although technically you _can_, by casting, and that would indeed break correctness, so you should know what you're doing. As for the `operator[]`, does one version call another, like in this question? – Sergei Tachenov Jan 05 '16 at 05:19
  • @SergeyTachenov You wrote "If it doesn't modify anything, it should accept a const pointer and return a non-const pointer." I should have written const pointer, not const object. I don't know whether operator[] calls the other, probably not since it's a one liner anyhow in the case of vector. The point is that it can, it's the classic example of that pattern of const overloaded functions returning potentially const qualified references/pointers. You make it out like having two const overloaded functions is strange, when it's actually extremely common. – Nir Friedman Jan 05 '16 at 15:25
  • @Nir, I get it. It's probably the whole `static` thing that confused me. – Sergei Tachenov Jan 05 '16 at 15:37

2 Answers2

2

The most important rule is that an interface function (public method, a free function other than one in a detail namespace, etc), should not cast away the constness of its input. Scott Meyer was one of the first to talk about preventing duplication using const_cast, here's a typical example (How do I remove code duplication between similar const and non-const member functions?):

struct C {
  const char & get() const {
    return c;
  }
  char & get() {
    return const_cast<char &>(static_cast<const C &>(*this).get());
  }
  char c;

};

This refers to instance methods rather than static/free functions, but the principle is the same. You notice that the non-const version adds const to call the other method (for an instance method, the this pointer is the input). It then casts away constness at the end; this is safe because it knows the original input was not const.

Implementing this the other way around would be extremely dangerous. If you cast away constness of a function parameter you receive, you are taking a big risk in UB if the object passed to you is actually const. Namely, if you call any methods that actually mutate the object (which is very easy to do by accident now that you've cast away constness), you can easily get UB:

C++ standard, section § 5.2.11/7 [const cast]

[ Note: Depending on the type of the object, a write operation through the pointer, lvalue or pointer to data member resulting from a const_cast that casts away a const-qualifier may produce undefined behavior. —end note ]

It's not as bad in private methods/implementation functions because perhaps you carefully control how/when its called, but why do it this way? It's more dangerous to no benefit.

Conceptually, it's often the case that when you have a const and non-const version of the same function, you are just passing along internal references of the object (vector::operator[] is a canonical example), and not actually mutating anything, which means that it will be safe either way you write it. But it's still more dangerous to cast away the constness of the input; although you might be unlikely to mess it up yourself, imagine a team setting where you write it the wrong way around and it works fine, and then someone changes the implementation to mutate something, giving you UB.

In summary, in many cases it may not make a practical difference, but there is a correct way to do it that's strictly better than the alternative: add constness to the input, and remove constness from the output.

Community
  • 1
  • 1
Nir Friedman
  • 15,634
  • 2
  • 33
  • 63
  • "const_cast immediately invokes UB because its casting away the constness of a const object"... nope, that's not UB. Actually trying to modify a `const` object is UB (via any method, including after using `const_cast`, but the cast itself is not modification). – Ben Voigt Jan 04 '16 at 20:28
  • @BenVoigt My mistake, your are correct. I'm curious then, suppose you have this non-const pointer to a const object. What if you now call a method, you will get the non-const overload of a method on a const object. Is this automatically UB? Or does it then in turn depend what the method does? Turtles all the way down? ;-) – Nir Friedman Jan 04 '16 at 21:01
  • I'm confident the spirit of what I wrote is still correct, I'd like to correct it to give it the precisely correct technical backing with your help. – Nir Friedman Jan 04 '16 at 21:02
0

I have actually only ever seen your first version before, so from my experience it is the more common idiom.

The first version seems correct to me while the second version can result in undefined behavior if (A) you pass an actual const object to the function and (B) the long code writes to that object. Given that in the first case the compiler will tell you if you're trying to write to the object I would never recommend option 2 as it is. You could consider a standalone function that takes/returns const however.

Mark B
  • 91,641
  • 10
  • 102
  • 179