views:

305

answers:

3

Hi,

In one of my projects, I have some classes that represent entities that cannot change once created, aka. immutable classes.

Example : A class RSAKey that represent a RSA key which only has const methods. There is no point changing the existing instance: if you need another one, you just create one.

My objects sometimes are heavy and I enforced the use of smart pointers to avoid deep copy.

So far, I have the following pattern for my classes:

class RSAKey : public boost::noncopyable, public boost::enable_shared_from_this<RSAKey>
{
    public:

    /**
     * \brief Some factory.
     * \param member A member value.
     * \return An instance.
     */
    static boost::shared_ptr<const RSAKey> createFromMember(int member);

    /**
     * \brief Get a member.
     * \return The member.
     */
    int getMember() const;

    private:

    /**
     * \brief Constructor.
     * \param member A member.
     */
    RSAKey(int member);

    /**
     * \brief Member.
     */
    const int m_member;
};

So you can only get a pointer (well, a smart pointer) to a const RSAKey. To me, it makes sense, because having a non-const reference to the instance is useless (it only has const methods).

Do you guys see any issue regarding this pattern ? Are immutable classes something common in C++ or did I just created a monster ?

Thank you for your advices !

+7  A: 

This seems like overkill. When you only have const members, the object can not be changed anyway. If you want to disallow copying, just make the copy constructor and the assignment operator private.

Michael Ulm
This is already done by inheriting from boost::noncopyable.
ereOn
boost:noncopyable is more explicit. Looking at the interface you'll immediately see the ": public boost::noncopyable" while you might not notice the private copy constructor and assignment operator.
f4
But that brings in the considerable coupling overhead of Boost, when a perfectly good idiom - hiding the copy constructor and copy assignment operator - already exists. What is the benefit to the programmer? (The benefits to the promulgators of Boost are obvious)
DannyT
I already use Boost **a lot** in this project (mainly for the `boost::shared_ptr<>`). Personnaly, I think inheriting from `boost::noncopyable` is more readable.
ereOn
If you don't want coupling with boost, creating your own "noncopyable" is pretty straightforward, but why reinvent the wheel?
stefaanv
Right on. I certainly don't want to reinvent a wheel that is a (distorted) reinvention of an existing technique!
DannyT
I disagree (with the overkill part, I won't go into the boost::noncopyable discussion): even if having only const methods/members guarantee that the object will not ever be changed, returning const pointer makes it more readable for the casual reader. I don't need to know whether the class is immutable or not: I cannot mutate it since I was handed a pointer to const.
David Rodríguez - dribeas
@DannyT: lol, I meant the opposite, but I can see how you interprete it. Actually, I really meant to create 1 noncopyable so your class can inherit from it, just as boost does for you.
stefaanv
+1  A: 

Looks good to me.

Marking every object const from the factory obviates marking every data member const, but in this example there is only one anyway.

Potatoswatter
@Potatoswatter, why is your answer a *community wiki* ? (I didn't even know answers could be *community wiki* on their own). Just wondering.
ereOn
@ereOn: Because I don't feel I'm contributing anything. If someone wants to add actual content to my post, feel free. It's more of a dummy so you can accept that you're right in the end.
Potatoswatter
A: 

As I dont have enough rep to comment Michael's answer..

About the decision of returning const ref - what if application developer uses const_cast<>? Should this be taken in consideration?

Fdr
no, this is undefined behaviour. if the caller wants nasal demons it's their call.
jk
Yeah. The use of const_cast usually indicates a design flaw anyway.
ereOn
I've use it in two situations:I've `const_cast<char *>( _some_std_string.c_str() )` to pass to `execv` and I've implemented a msvc version of `__sync_fetch_and_add` with ` __sync_fetch_and_add(volatile long* a, long add) { long a_ = *a; return InterlockedCompareAndExcahnge(a,_a+add,_a);}` since InterlockedCompareAndExcahnge requires (volatile long *, long, long)
KitsuneYMG