4
\$\begingroup\$

Here is my implementation of SmartPointer. It is intrusive and tiny. How can I improve it?

For example, constexpr, nonexcept, try-catch-throw, and etc.

smartpointer.h

/* 
* file name : smartpointer.h
*/
#ifndef __SMARTPOINTER_H__
#define __SMARTPOINTER_H__

#define COMPARE(_op_)                                           \
bool operator _op_ (const SmartPointer& o) const {              \
    return mPointer _op_ o.mPointer;                            \
}                                                               \
bool operator _op_ (const T* o) const {                         \
    return mPointer _op_ o;                                     \
} 

template <typename T> 
class SmartPointer {
public:
    SmartPointer():mPointer(NULL) {std::cout <<"Create null smart pointer."<< std::endl;}   
    SmartPointer(T *p):mPointer(p) {
        std::cout <<"Create smart pointer at "<<static_cast<const void*>(p)<<std::endl;
        if (mPointer) mPointer->incRefCount();
    }     
    ~SmartPointer(){
        std::cout << "Release smart pointer at "<<static_cast<const void*>(mPointer)<<std::endl;
        if (mPointer && mPointer->decRefCount()==0) delete mPointer;
    }
    SmartPointer(const SmartPointer &other):mPointer(other.mPointer) {
        std::cout <<"Copy smart pointer at "<<static_cast<const void*>(other.mPointer)<<std::endl;
       if(mPointer) mPointer->incRefCount();
    }             
   SmartPointer &operator = (const SmartPointer &other) {
       T *temp(other.mPointer);
       if (temp) temp->incRefCount();
       if (mPointer && mPointer->decRefCount()==0) delete mPointer;
       mPointer = temp;  
       return *this;
   } 

    T&  operator* () const {return *mPointer;}
    T* operator-> () const {return mPointer;}


    COMPARE(==);
    COMPARE(!=);
private:
    T *mPointer;
};

class RefBase   
{   
    public:   
        RefBase() : mCount(0){ }   
        void incRefCount(){   
            mCount++;   
        }   
        int decRefCount(){   
            return --mCount;
        }      
        int getRefCount() const {   
            return mCount;   
        }  

        virtual ~RefBase(){};
    private:   
        int mCount;   
};   
#endif // __SMARTPOINTER_H__

sptestcase4.cpp (test case)

/* 
* file name : sptestcase4.cpp
*/

#include <iostream>
#include "smartpointer.h"

class SomeClass: public RefBase{
public:
    SomeClass(){std::cout << "SomeClass default constructor !"<<std::endl;}
    ~SomeClass(){std::cout << "SomeClass deconstructor !"<<std::endl;}
    void func(){std::cout << "SomeClass func()" <<std::endl;}
};

class OtherClass: public RefBase{
public:
    OtherClass(){std::cout << "OtherClass default constructor !"<<std::endl;}
    ~OtherClass(){std::cout << "OtherClass deconstructor !"<<std::endl;}
    void foo(){std::cout << "OtherClass foo()" <<std::endl;}
};


void testcase4_1(void)
{
    std::cout << "=======testcase4_1=========" <<std::endl;
    SmartPointer<SomeClass> spsomeclass = new SomeClass();
    (*spsomeclass).func();
    spsomeclass->func();
    std::cout << "==========================" <<std::endl;
}

void testcase4_2(void)
{
    std::cout << "=======testcase4_2=========" <<std::endl;
    SomeClass *psomeclass = new SomeClass();
    SmartPointer<SomeClass> spsomeclass = psomeclass;

    SmartPointer<OtherClass> spotherclass = new OtherClass();
    SmartPointer<OtherClass> spotherclass2 = spotherclass;

    if (spsomeclass == NULL) std::cout<< "spsomeclass is NULL pointer" << std::endl;
    if (spotherclass != NULL) std::cout<< "spotherclass is not NULL pointer" << std::endl;
    if (spsomeclass == psomeclass)  
       std::cout<< "spsomeclass and psomeclass are same pointer" << std::endl;
    if (spsomeclass != psomeclass)  
       std::cout<< "spsomeclass and psomeclass are not same pointer" << std::endl;
//     if (spsomeclass != spotherclass) // ERROR !
//        std::cout<< "spsomeclass and spotherclass are not same pointer" << std::endl;
//     if (spsomeclass == spotherclass) // ERROR !
//        std::cout<< "spsomeclass and spotherclass are same pointer" << std::endl;
    if (spotherclass == spotherclass2) std::cout<< "spotherclass and spotherclass2 are same pointer" << std::endl;
    if (spotherclass != spotherclass2) std::cout<< "spotherclass and spotherclass2 are not same pointer" << std::endl;    
    std::cout << "==========================" <<std::endl;

}

int main(void)
{
    testcase4_1();
    testcase4_2();
    return 0;
}

enter image description here

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Not trying to invalidate your work, but you should know that the stl provides std::enable_shared_from_this in order to provide that very functionality. \$\endgroup\$
    – user128454
    Sep 8, 2017 at 4:35

1 Answer 1

1
\$\begingroup\$

By using identifiers that start with an underscore, you are invoking undefined behavior. Just remove the underscores.

At the end of the header file, you should undefine the COMPARE macro. Or at least consider removing the macro completely.

Your placement of the & is inconsistent.

Your header file must compile on its own, i.e. even when it is included before <iostream>.

\$\endgroup\$
4
  • \$\begingroup\$ sorry, there is something wrong when I copy my codes \$\endgroup\$
    – Chen Li
    Sep 8, 2017 at 1:54
  • \$\begingroup\$ I have edited my question, thanks for your advice \$\endgroup\$
    – Chen Li
    Sep 8, 2017 at 2:06
  • 1
    \$\begingroup\$ Re: underscores, that's not correct, only identifiers starting with two consecutive underscores, or a single underscore followed by a capital letter are reserved. \$\endgroup\$
    – user128454
    Sep 8, 2017 at 4:32
  • \$\begingroup\$ @Frank "Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace." See [global.names] in the C++11 standard. Only holds for the global namespace, though. \$\endgroup\$
    – Zeta
    Mar 24, 2018 at 7:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.