C++ TemplateClass

Question

I have written a (dynamic) TemplateArray class, for purposes to be included as a baseline for a library I am working on, and I would like feedback on any of the following:

• How the code can be improved (even if it's just your subjective opinion)?
• How the code can be made more efficient?
• How the functions of the class can be made more intuitive and user-friendly (does it operate like how you anticipate it to)?

namespace TL
{

template<typename TemplateItem>
class TemplateArray
{

protected:

        TemplateItem *Array;
        SIZE_TYPE Size;

        void Clear(){Array = NULL; Size = 0;}

    public:

        const bool IsEmpty() const { return ((Array == NULL) && (Size == 0)); }
        const bool IsValid() const { return ((Array != NULL) && (Size != 0)); }

        const bool operator!() const {return IsEmpty();}
        operator const bool () const {return IsValid();}

        const SIZE_TYPE GetSize() const {return Size;}

        const bool SetSize(const SIZE_TYPE S)
        {
            ERRORCLASSFUNCTION(TemplateArray,SetSize,S < 1,RETURN_BOOL)
            if(!IsEmpty())
            {
                Close();
            }
            ERRORCLASSFUNCTION(TemplateArray,SetSize,!CREATEB(Array,S),RETURN_BOOL)
            Size = S;
            return true;
        }

        const bool CopyArray(const TemplateItem Arr[], const SIZE_TYPE S)
        {
            ERRORCLASSFUNCTION(TemplateArray,CopyArray,S < 1,RETURN_BOOL);
            ERRORCLASSFUNCTION(TemplateArray,CopyArray,S > Size,RETURN_BOOL);

            SIZE_TYPE Temp = 0;
            while(Temp < S)
            {
                Array[Temp] = Arr[Temp];
                Temp++;
            }
            return true;
        }

        const bool SetArray(const TemplateItem Arr[], const SIZE_TYPE S)
        {
            ERRORCLASSFUNCTION(TemplateArray,SetArray,S < 1,RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,SetArray,!SetSize(S),RETURN_BOOL)
            SIZE_TYPE Temp = 0;
            while(Temp < S)
            {
                Array[Temp] = Arr[Temp];
                Temp++;
            }
            return true;
        }

        const bool SetArray(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,SetArray,!ItemCopy,RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,SetArray,!SetArray(ItemCopy.Array,ItemCopy.Size),RETURN_BOOL)
            return true;
        }

        const bool SetArray(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,SetArray,ItemCopy.empty(),RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,SetArray,!SetArray(&ItemCopy[0],ItemCopy.size()),RETURN_BOOL)
            return true;
        }

        TemplateItem * const GetArray() { return Array; }
        const TemplateItem * const GetArray() const { return Array; }

        void TransferFrom(TemplateArray &ItemCopy)
        {
            Close();
            Array = ItemCopy.Array;
            Size = ItemCopy.Size;
            ItemCopy.Array = NULL;
            ItemCopy.Size = 0;
        }

        const bool Reverse()
        {
            ERRORCLASSFUNCTION(TemplateArray,Reverse,!IsValid(),RETURN_BOOL)
            TemplateArray<TemplateItem> Temp;
            ERRORCLASSFUNCTION(TemplateArray,Reverse,!Temp.SetSize(Size),RETURN_BOOL)
            TemplateItem *C_Ptr = Array+(Size-1), *C_Ptr_2 = Temp.GetArray();

            while(C_Ptr != Array)
            {
                *C_Ptr_2 = *C_Ptr;
                C_Ptr--; C_Ptr_2++;
            }
            *C_Ptr_2 = *C_Ptr;
            TransferFrom(Temp);
            return true;
        }

        const bool Compare(const TemplateItem Arr[], const SIZE_TYPE S)
        {
            if(!IsValid())
            {
                return false;
            }

            ERRORCLASSFUNCTION(TemplateArray,Compare,S < 1,RETURN_BOOL)
            SIZE_TYPE Temp = 0;

            while(Temp < S)
            {
                if(Array[Temp] != Arr[Temp]){return false;}
                Temp++;
            }

            return true;
        }

        const bool Compare(const TemplateArray &ItemCopy)
        {
            if(Size != ItemCopy.Size)
            {
                return false;
            }

            if(IsEmpty())
            {
                return true;
            }

            return Compare(ItemCopy.Array,ItemCopy.Size);
        }

        const bool Compare(const std::vector<TemplateItem> &ItemCopy)
        {
            if(ItemCopy.size() != Size)
            {
                return false;
            }

            if(ItemCopy.empty())
            {
                return true;
            }

            return Compare(&ItemCopy[0],ItemCopy.size());
        }

        const bool Append(const TemplateItem Data[], const SIZE_TYPE S)
        {
            ERRORCLASSFUNCTION(TemplateArray,Append,Data == NULL,RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,Append,S < 1,RETURN_BOOL)
            TemplateArray<TemplateItem> Temp;
            ERRORCLASSFUNCTION(TemplateArray,Append,!Temp.SetSize(Size+S),RETURN_BOOL)
            SIZE_TYPE Temp2 = 0;
            while(Temp2 < Size)
            {
                Temp.Array[Temp2] = Array[Temp2];
                Temp2++;
            }

            Temp2 = 0;
            while(Temp2 < S)
            {
                Temp.Array[Temp2+Size] = Data[Temp2];
                Temp2++;
            }
            TransferFrom(Temp);
            return true;
        }

        //Tested
        const bool Append(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,Append,!ItemCopy.IsValid(),RETURN_BOOL)
            return Append(ItemCopy.GetArray(),ItemCopy.GetSize());
        }

        const bool Append(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,Append,ItemCopy.empty(),RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,Append,!Append(&ItemCopy[0],ItemCopy.size()),RETURN_BOOL)
            return true;
        }

        const bool Append(const TemplateItem &ItemCopy)
        {
            return Append(&ItemCopy,1);
        }

        const bool Remove(const TemplateItem Data[], const SIZE_TYPE S)
        {
            ERRORCLASSFUNCTION(TemplateArray,Append,Data == NULL,RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,Remove,S < 1,RETURN_BOOL)
            if(!IsValid()){return false;}
            if(S > Size){return false;}

            SIZE_TYPE Temp = 0, Temp2 = 0;

            do
            {
                Temp2 = 0;
                while(Temp < Size)
                {
                    if(Array[Temp] == Data[Temp2])
                    {
                        break;
                    }
                    Temp++;
                }

                while(Temp < Size)
                {
                    if(Array[Temp] != Data[Temp2])
                    {
                        break;
                    }

                    Temp++; Temp2++;

                    if(Temp2 == S)
                    {
                        break;
                    }
                }

                if(Temp2 == S)
                {
                    TemplateArray<TemplateItem> DataTemp;

                    ERRORCLASSFUNCTION(TemplateArray,operator-=,!DataTemp.SetSize(Size-S),RETURN_BOOL)
                    Temp2 = Temp - S;
                    Temp = 0;

                    while(Temp < Temp2)
                    {
                        DataTemp.Array[Temp] = Array[Temp];
                        Temp++;
                    }

                    Temp2 += S;

                    while(Temp2 < Size)
                    {
                        DataTemp.Array[Temp] = Array[Temp2];
                        Temp++; Temp2++;
                    }

                    TransferFrom(DataTemp);
                    return *this;
                }

            }while(Temp < Size);

            return *this;
        }

        const bool Remove(const std::vector<TemplateArray> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,Remove,ItemCopy.empty(),RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,Remove,!Remove(&ItemCopy[0],ItemCopy.size()),RETURN_BOOL)
            return true;
        }

        const bool Remove(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,Remove,!IsValid(),RETURN_BOOL)
            ERRORCLASSFUNCTION(TemplateArray,Remove,!Remove(ItemCopy.Array,ItemCopy.Size),RETURN_BOOL)
            return *this;
        }

        const bool Remove(const TemplateItem &ItemCopy)
        {
            return Remove(&ItemCopy,1);
        }

        void Reset()
        {
            if(Array != NULL)
            {
                DELETEB(Array);
            }
            Open();
        }
        void Open(){ Clear(); }
        void Close()
        {
            if(Array != NULL)
            {
                DELETEB(Array);
            }
            Clear();
        }

        //Tested
        TemplateArray(){Open();}

        ~TemplateArray(){Close();}

        //Tested
        TemplateArray(const TemplateItem Data[], const SIZE_TYPE S)
        {
            Open();
            ERRORCLASSFUNCTION(TemplateArray,TemplateArray,!SetArray(Data,S),)
        }

        //Tested
        TemplateArray(const TemplateArray &ItemCopy)
        {
            Open();
            ERRORCLASSFUNCTION(TemplateArray,TemplateArray,!SetArray(ItemCopy),)
        }

        TemplateArray(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,TemplateArray,!SetArray(ItemCopy),RETURN_VOID)
        }

        operator const SIZE_TYPE () const {return Size;}

        operator const std::vector<TemplateItem> () const
        {
            std::vector<TemplateItem> Temp;
            if(!IsValid())
            {
                return Temp;
            }

            try
            {
                Temp.resize(Size);
            }
            catch(...)
            {
                std::vector<TemplateItem> Temp2;
                ERRORCLASSFUNCTION(TemplateArray,std::vector,true,return Temp2;)
            }

            SIZE_TYPE Temp2 = 0;

            while(Temp2 < Size)
            {
                Temp[Temp2] = Array[Temp2];
                Temp2++;
            }

            return Temp;
        }

        const bool operator!=(const TemplateArray &ItemCopy){return !((*this) == ItemCopy);}
        const bool operator==(const TemplateArray &ItemCopy)
        {
            return Compare(ItemCopy);
        }

        const bool operator==(const std::vector<TemplateItem> &ItemCopy)
        {
            return Compare(ItemCopy);
        }

        TemplateArray &operator+=(const TemplateItem &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator+=,!Append(ItemCopy),RETURN_THIS)
            return *this;
        }

        //Tested
        TemplateArray &operator+=(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator+=,!Append(ItemCopy),RETURN_THIS)
            return *this;
        }

        //Tested
        TemplateArray &operator+=(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator+=,!Append(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateArray &operator-=(const TemplateItem &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator-=,!Remove(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateArray &operator-=(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator-=,!Remove(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateArray &operator-=(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator-=,!Remove(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateArray &operator=(const std::vector<TemplateItem> &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator=,!SetArray(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateArray &operator=(const TemplateArray &ItemCopy)
        {
            ERRORCLASSFUNCTION(TemplateArray,operator=,!SetArray(ItemCopy),RETURN_THIS)
            return *this;
        }

        TemplateItem &operator[](const SIZE_TYPE S){ return Array[S]; }
        const TemplateItem &operator[](const SIZE_TYPE S) const { return Array[S]; }
};

}

A working example you can paste directly into your compiler is also available here.

The class has to support the following:

To minimise bug tracing times:

• Vocal error reporting.
• Traceable error reporting.
• Fail-safes (quit on first problem attitude).
• Functions that primarily return bool where possible.

For compatibility with other classes:

• Return and accept a standard data-type (in this case, std::vector).

The class should support the following requirements:

• User-friendliness. User should not have to think when using the class.
• Compatibility with future, potentially unknown classes and sub-classes.

Please ignore the macro, printf usage and the fact the functions are inside the class. It's just my personal preference. Please avoid suggesting using std::string/std::vector (this is a really old cliche now), as standard classes are generic and don't offer the specific functionality a custom built class can (i.e. bool returning, error-reporting/tracing functions), although it does offer compatibility with such classes.

Answer 1

How the code can be improved (even if it's just your subjective opinion)?

Replace the macros with readable code.
They make it much harder to read and understand than it needs to be.

Your object does not provide any strong guarantees (which is what I would expect from a container). Either the operation succeeded or the operation fails and the object is left in its original state. Your array does not provide this. If it fails the original state is usually lost.

This breaks your own rule of User-friendliness. User should not have to think when using the class. Now they do have to think. O crap that last operation just lost all my data I can;t even dump my current state for debugging.

You don't seem to check for self assignment (not that I can see I could be wrong but the code is very hard to read). Thus you will break as you delete the array you are copying (yourself) and then read from the now dead array illiciting undefined behavior.

How the code can be made more efficient?

That will take a long time.
But it looks like you have done a basic job of getting it working.

It may improve the use of the swap method which can be useful in a lot of situations and the default implementation of swap is not very efficient for any container.

You do not re-use the array. You always delete and allocate. That can be very in-efficient. It may be nicer to check if the array being copied over this one is smaller and if so re-use the array just move/copy the elements into this array.

You don't pre-allocate memory for possible expansion in the near future and thus fall into the trap of deleting and re-allocating the array for every push onto the back no matter how big it is.

How the functions of the class can be made more intuitive and user-friendly (does it operate like how you anticipate it to)?

This is where your class is horrible. It is basically the most unfriendly container class I have seen and can not be used with any of the standard algorithms, which makes it practically unusable.

Generally all-round friendly coder advice to a non-expert.

• Get rid of the macros replace them with function calls.
• Implement iterators. (so you can use the standard algorithms)
• Provide better protection for your user.
  • Operations that fail should leave the object unchanged.

I will go to the cliche of saying I don't see any benefit of using this over std::vector (and I do see lots of negatives: Two of the big ones are ease of use and efficiency (Both contradicting your own design goals)).

Answer 2

All your methods have corresponding functionality with std::vector. I suggest you write your class as a wrapper around std::vector.

   template<typename TemplateItem>
class TemplateArray
{
    public:
        // std::vector::empty
        const bool IsEmpty() const { return ((Array == NULL) && (Size == 0)); }

        // unnecessary
        const bool IsValid() const { return ((Array != NULL) && (Size != 0)); }
        const bool operator!() const {return IsEmpty();}
        operator const bool () const {return IsValid();}

        // std::vector::size
        const SIZE_TYPE GetSize() const {return Size;}

        // std::vector::resize
        const bool SetSize(const SIZE_TYPE S)

        // std::copy
        const bool CopyArray(const TemplateItem Arr[], const SIZE_TYPE S)
        const bool SetArray(const TemplateItem Arr[], const SIZE_TYPE S)
        const bool SetArray(const TemplateArray &ItemCopy)
        const bool SetArray(const std::vector<TemplateItem> &ItemCopy)
        TemplateItem * const GetArray() { return Array; }
        const TemplateItem * const GetArray() const { return Array; }

        // move constructor
        void TransferFrom(TemplateArray &ItemCopy)

        // std::reverse
        const bool Reverse()

        // std::vector::operator==
        const bool Compare(const TemplateItem Arr[], const SIZE_TYPE S)
        const bool Compare(const TemplateArray &ItemCopy)
        const bool Compare(const std::vector<TemplateItem> &ItemCopy)

        // std::vector::push_back
        const bool Append(const TemplateItem Data[], const SIZE_TYPE S)
        const bool Append(const TemplateArray &ItemCopy)
        const bool Append(const std::vector<TemplateItem> &ItemCopy)
        const bool Append(const TemplateItem &ItemCopy)

        // std::vector::erase + std::remove.
        const bool Remove(const TemplateItem Data[], const SIZE_TYPE S)
        const bool Remove(const std::vector<TemplateArray> &ItemCopy)
        const bool Remove(const TemplateArray &ItemCopy)
        const bool Remove(const TemplateItem &ItemCopy)

        // std::vector::clear
        void Clear()
        void Reset()

        // A bit wierd, but i guess you want something like std::vector::clear + std::vector::shrink_to_fit
        void Open()
        void Close()

        // Nice to have i guess, std::vector::push_back
        TemplateArray &operator+=(const TemplateItem &ItemCopy)

        // std::vector::operator[]
        TemplateItem &operator[](const SIZE_TYPE S){ return Array[S]; }
        const TemplateItem &operator[](const SIZE_TYPE S) const { return Array[S]; }
};