7

I have the following class

class CItem { public: CItem(CRegistry &Registry) _Registry(Registry) {Registry.Register();} ~CItem() {_Registry.Unregister()}; private: CRegistry &_Registry; }

After a while it turns out that not all CItem objects need to be registered so I need a version of CItem which does not requires Registry in constructor (and of course of the registration code). How can I implement this? The only solution I can see here is to get and keep Registry as a pointer. Is there more elegant solution, like using templates, etc (I don't like switching from reference to pointer)?

Improve this question
4
  • 3
    BTW, names with leading underscores are reserved by the standard. Commented Jun 9, 2009 at 13:21
  • 4
    Particularly names that begin with an underscore and an uppercase letter! Commented Jun 9, 2009 at 13:29
  • To elaborate: names beginning underscore followed by uppercase letter are reserved everywhere. Other names beginning underscore are reserved only in the global namespace, so can be used for member variables if you insist (and don't mind the small chance that they will hide some implementation-specific symbol). 17.4.3.1.2. Commented Jun 9, 2009 at 13:46
  • 1
    Read this: stackoverflow.com/questions/228783/… Commented Jun 9, 2009 at 15:06

9 Answers 9

Reset to default
10

If you want to keep a single class, just change the attribute into a raw pointer and allow it to be null. As Neil points out, there is a widespread unjustified jihad against raw pointers that is not fully justified. Use a raw pointer and clearly document (comment) that the object holds no ownership of the pointed memory so that no one feels like adding a delete in your destructor at a later time.

All other solutions will be worse than using a pointer internally. It is an implementation detail. Also consider whether it makes sense. Your code will no longer be able to assume that the pointer is valid and it will complicate the logic inside your class.

class CItem { public: CItem(CRegistry &Registry) : _Registry(&Registry) {Registry->Register();} CItem() : _Registry(0) {} ~CItem() { if ( _Registry ) _Registry->Unregister(); } private: CRegistry *_Registry; // Pointer is not owned. Do not delete! };

As a last note: do not prefix attributes with a single underscore as they are reserved by the standard for C++ implementations (compiler and standard libraries)

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

"do not prefix attributes with a single underscore" and in case it isn't obvious, don't use multiple underscores either. Names containing a double underscore anywhere in them are reserved for all uses.
To be more explicit about the optional semantics, you could use boost::optional rather than a raw pointer. But otherwise I agree, for "built-in" optional support, pointers do the trick nicely.
5

The only other alternative would be to create a CItem base class and then derive ItemWithRef (which has a reference) and ItemWithoutRef from it. But ir's much easier and clearer to use a pointer.

From the number of questions regarding references as members, it seems that somewhere, someone is spreading the view that references are good and pointers are bad. This is simply not the case, particularly when it comes to data members.

And just to clarify the underscore thing:

  • names that begin with an underscore and a lowervcase letter are reserved for the C++ compile/library writers when they occur at namespace (i.e. outside of classes) scope

  • names thatb begin with an underscore and an uppercase letter, or contain two consecutive underscores are unconditionally reserved for the compiler/library writers - you must not use them in your own code

Improve this answer

Comments

2

By having a reference member, you are clearly saying that a registry is needed for every CItem (because a reference must be bound to a valid object, and every CItem has a reference member). The direct way to implement an optional CRegistry is to use boost::optional (this is safer and clearer than the NULL pointer idiom). Alternatively, the Null Object Pattern lets you have a CNullRegistry class which implements registration, deregistration, and other functions, as no-ops. Then make the constructor parameter default to a CNullRegistry object.

However, you might like to consider a higher level approach which clearly delineates registered CItems from unregistered ones. As other answers suggested, inheritance and template specialization both provide mechanisms for that. The advantage is that your design can now depend on the "I'm registered" invariant.

Improve this answer

Comments

1
  1. Use inheritance: Make a base class CItem and derive CRegisteredItem from this.
  2. Use pointers (and an overloaded constructor)
Improve this answer

Comments

1

One obvious alternative to a pointer is a private: static CRegistry selfRegistered;. Youn can then write CItem::CItem() : Registry(selfRegistered) { }

Improve this answer

Comments

1

Could you create another constructor that doesn't take a CRegistry as a parameter:

CItem();

that initializes _Registry to a static "zombie" value? Probably less elegant than using a pointer or subclassing?

Improve this answer

2 Comments

To do that you have to be able to create a "null" Registry object, which will probably entail changes to the Registry class. This is s very bad solution, IMHO.
True. Not one that I personally would implement. I'd go with a private pointer.
1

Make CRegistry an abstract class:

 class CRegistry { public: virtual void Register(const CItem& Item) = 0; virtual void Unregister(const CItem& Item) = 0; };

Then derive two implementations

 class CNoopRegistry : public CRegistry { public: virtual void Register(const CItem& Item) {} virtual void Unregister(const CItem& Item) {} }; class CWorkingRegistry : public CRegistry { public: virtual void Register(const CItem& Item) { /* do something useful */ } virtual void Unregister(const CItem& Item) { /* do something useful */ } };

and pass whichever Instance you need to CItem's constructor.

Improve this answer

Comments

0

You could make CRegistry a singleton (or simply just a standalone class) and decide in the CItem constructor whether you want to register that particular instance. This decouples the items from the registry and IMHO makes it easier to change things in the future.

Improve this answer

Comments

0

You can probably use template specialization like this:

class CRegistry { public: void Register(){} void Unregister(){} }; template <class RegistryType> class CItem { public: CItem(){} ~CItem() {} }; template<> class CItem<CRegistry> { public: CItem(CRegistry &Registry_in): Registry(Registry_in) {Registry.Register();} ~CItem() {Registry.Unregister();} private: CRegistry& Registry; }; int main() { CRegistry c1; CItem<CRegistry> it(c1); CItem<int> it2; return 0; }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.