1

I am reviewing an approach I see commonly used in storing objects (e.g. a socket client object). Namely, usage of a static container to hold the objects. Said objects are created by some helper function as follows:

create_client(params) {
  ...
  client* cl = new client(...);
  return cl;
}

The thing that confused me at first was function called like this:

if (!create_client (...))
   //generate error message

i.e. a copy of the pointer just seems to be thrown away.

But on investigation I see this in the client constructor:

client::client(...) {
    ...
    coll[id] = this;
}

Where coll is a map of id to a pointer to the object. But anyway, just a collection. coll is static (not sure if that is relevant).

Is there a name for this idiom? Is it good practice?

  • In your second code block, did you mean if (!create_client(...))? – Dan Pichelman Jun 18 '14 at 16:32
  • @DanPichelman - if not created correctly print error. Its not important to question. – user619818 Jun 18 '14 at 16:44
  • Dan means that you define the function as client_create and later use it as create_client. – SJuan76 Jun 18 '14 at 17:00
  • Well spotted! I have edited. – user619818 Jun 18 '14 at 17:01
3

Not so familiar with C++, but I do not like the assignment into the static structure from the constructor, because:

1) Introduces a dependency: now the contained class needs to know about the containing instance. You cannot reuse the class without either rewritting or using the same structure.

2) It causes a reference leak from the constructor, since the reference to the class is available to other threads before the constructor has finished executing.

At the very least, I would have moved the instance assignment from the constructor to the create_client function.

4

Is there a name for this idiom? Is it good practice?

No, and no, unless you count "Terrible" as an idiom name. There is no such thing as idiomatic mutable global state. Idioms exist to avoid such crap.

  • "Idioms exist to avoid such crap." Unless it's the singleton idiom (cough). Now where's the mouthwash? – Thomas Eding Jun 18 '14 at 22:05
  • 4
    Singleton is not an idiom. It is an idiot filter. – DeadMG Jun 18 '14 at 22:11
0

This thing is called a multiton, so, yes, it is an idiom. It's not a good practice, though.

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.