Refactored CBotString to use std::string and CBotStringArray to use std::vector

src/CBot/CBotString.cpp

@@ -25,6 +25,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <algorithm>
+#include <boost/algorithm/string.hpp>
 
 //Map is filled with id-string pars that are needed for CBot language parsing
 const std::map<EID,const char *> CBotString::s_keywordString =
@@ -120,216 +121,97 @@ const std::map<EID,const char *> CBotString::s_keywordString =
 
 CBotString::CBotString()
 {
-    m_ptr = nullptr;
+    m_str = "";
-    m_lg  = 0;
 }
 
 CBotString::~CBotString()
 {
-    delete[] m_ptr;
-    m_ptr = nullptr;
 }
 
 
 CBotString::CBotString(const char* p)
 {
-    m_lg = strlen(p);
+    m_str = p;
+}
 
-    m_ptr = nullptr;
+CBotString::CBotString(const std::string &p)
-    if (m_lg>0)
+{
-    {
+    m_str = p;
-        m_ptr = new char[m_lg+1];
-        strcpy(m_ptr, p);
-    }
 }
 
 CBotString::CBotString(const CBotString& srcString)
 {
-    m_lg = srcString.m_lg;
+    m_str = srcString.m_str;
-
-    m_ptr = nullptr;
-    if (m_lg>0)
-    {
-        m_ptr = new char[m_lg+1];
-        strcpy(m_ptr, srcString.m_ptr);
-    }
 }
 
 
 
-
 int CBotString::GetLength()
 {
-    if (m_ptr == nullptr) return 0;
+    return m_str.length();
-    return strlen( m_ptr );
 }
 
 
 
 CBotString CBotString::Left(int nCount) const
 {
-    char    chain[2000];
+    return CBotString(m_str.substr(0, nCount));
-
-    // clamp nCount to correct value
-    if(nCount < 0) nCount = 0;
-    if(nCount > m_lg) nCount = m_lg;
-
-    int i;
-    for (i = 0; i < m_lg && i < nCount && i < 1999; ++i)
-    {
-        chain[i] = m_ptr[i];
-    }
-    chain[i] = 0 ;
-
-    return CBotString(chain);
 }
 
 CBotString CBotString::Right(int nCount) const
 {
-    char chain[2000];
+    return CBotString(m_str.substr(m_str.length()-nCount, std::string::npos));
-
-    // clamp nCount to correct value
-    if(nCount < 0) nCount = 0;
-    if(nCount > m_lg) nCount = m_lg;
-
-    int i = m_lg - nCount;
-    if ( i < 0 ) i = 0;
-
-    int j;
-    for (j = 0 ; i < m_lg && i < 1999; ++i)
-    {
-        chain[j++] = m_ptr[i];
-    }
-    chain[j] = 0 ;
-
-    return CBotString(chain);
 }
 
 CBotString CBotString::Mid(int start, int lg)
 {
-    CBotString res;
+    return CBotString(m_str.substr(start, lg));
-
-    if (lg == -1) lg = 2000;
-
-    // clamp start to correct value
-    if (start < 0) start = 0;
-    if (start >= m_lg) return res;
-
-    int remaining = m_lg - start;
-    if (lg > remaining) lg = remaining;
-    if (lg < 0) lg = 0;
-
-    char* p = new char[m_lg+1];
-    strcpy(p, m_ptr+start);
-    p[lg] = 0;
-
-    res = p;
-    delete[] p;
-    return res;
 }
 
 int CBotString::Find(const char c)
 {
-    for (int i = 0; i < m_lg; ++i)
+    std::size_t pos = m_str.find(c);
-    {
+    return pos != std::string::npos ? pos : -1;
-        if (m_ptr[i] == c) return i;
-    }
-    return -1;
 }
 
 int CBotString::Find(const char * lpsz)
 {
-    int l = strlen(lpsz);
+    std::size_t pos = m_str.find(lpsz);
-
+    return pos != std::string::npos ? pos : -1;
-    for (size_t i = 0; static_cast<int>(i) <= m_lg-l; ++i)
-    {
-        for (size_t j = 0; static_cast<int>(j) < l; ++j)
-        {
-            if (m_ptr[i+j] != lpsz[j]) goto bad;
-        }
-        return i;
-bad:;
-    }
-    return -1;
 }
 
 int CBotString::ReverseFind(const char c)
 {
-    int i;
+    std::size_t pos = m_str.rfind(c);
-    for (i = m_lg-1; i >= 0; --i)
+    return pos != std::string::npos ? pos : -1;
-    {
-        if (m_ptr[i] == c) return i;
-    }
-    return -1;
 }
 
 int CBotString::ReverseFind(const char * lpsz)
 {
-    int i, j;
+    std::size_t pos = m_str.rfind(lpsz);
-    int l = strlen(lpsz);
+    return pos != std::string::npos ? pos : -1;
-
-    for (i = m_lg-l; i >= 0; --i)
-    {
-        for (j = 0; j < l; ++j)
-        {
-            if (m_ptr[i+j] != lpsz[j]) goto bad;
-        }
-        return i;
-bad:;
-    }
-    return -1;
 }
 
 void CBotString::MakeUpper()
 {
-    for (size_t i = 0; static_cast<int>(i) < m_lg && static_cast<int>(i) < 1999 ; ++i)
+    boost::to_upper(m_str);
-    {
-        char c = m_ptr[i];
-        if ( c >= 'a' && c <= 'z' ) m_ptr[i] = c - 'a' + 'A';
-    }
 }
 
 void CBotString::MakeLower()
 {
-    for (size_t i = 0; static_cast<int>(i) < m_lg && static_cast<int>(i) < 1999 ; ++i)
+    boost::to_lower(m_str);
-    {
-        char    c = m_ptr[i];
-        if ( c >= 'A' && c <= 'Z' ) m_ptr[i] = c - 'A' + 'a';
-    }
 }
 
 bool CBotString::LoadString(unsigned int id)
 {
-    const char * str = nullptr;
+    m_str = MapIdToString(static_cast<EID>(id));
-    str = MapIdToString(static_cast<EID>(id));
+    return !m_str.empty();
-    if (m_ptr != nullptr)
-        delete[] m_ptr;
-
-    m_lg = strlen(str);
-    m_ptr = nullptr;
-    if (m_lg > 0)
-    {
-        m_ptr = new char[m_lg+1];
-        strcpy(m_ptr, str);
-        return true;
-    }
-    return false;
 }
 
 
 const CBotString& CBotString::operator=(const CBotString& stringSrc)
 {
-    delete[] m_ptr;
+    m_str = stringSrc.m_str;
-    m_ptr = nullptr;
-
-    m_lg = stringSrc.m_lg;
-
-    if (m_lg > 0)
-    {
-        m_ptr = new char[m_lg+1];
-        strcpy(m_ptr, stringSrc.m_ptr);
-    }
-
     return *this;
 }
 
@@ -342,174 +224,114 @@ CBotString CBotString::operator+(const CBotString& stringSrc)
 
 const CBotString& CBotString::operator=(const char ch)
 {
-    delete[] m_ptr;
+    m_str = ch;
-
-    m_lg = 1;
-
-    m_ptr = new char[2];
-    m_ptr[0] = ch;
-    m_ptr[1] = 0;
-
     return *this;
 }
 
 const CBotString& CBotString::operator=(const char* pString)
 {
-    delete[] m_ptr;
-    m_ptr = nullptr;
-
     if (pString != nullptr)
-    {
+        m_str = pString;
-        m_lg = strlen(pString);
+    else
-
+        m_str.clear();
-        if (m_lg != 0)
-        {
-            m_ptr = new char[m_lg+1];
-            strcpy(m_ptr, pString);
-        }
-    }
-
     return *this;
 }
 
 
 const CBotString& CBotString::operator+=(const char ch)
 {
-    char* p = new char[m_lg+2];
+    m_str += ch;
-
-    if (m_ptr != nullptr) strcpy(p, m_ptr);
-    p[m_lg++] = ch;
-    p[m_lg]   = 0;
-
-    delete[] m_ptr;
-
-    m_ptr = p;
-
     return *this;
 }
 
 const CBotString& CBotString::operator+=(const CBotString& str)
 {
-    char* p = new char[m_lg+str.m_lg+1];
+    m_str += str.m_str;
-
-    //-- Check if the pointer is not null befor trying to copy it
-    if(m_ptr != nullptr)
-    {
-        strcpy(p, m_ptr);
-    }
-
-    char* pp = p + m_lg;
-
-    //-- Check if the pointer is not null befor trying to copy it
-    if(str.m_ptr != nullptr)
-    {
-        strcpy(pp, str.m_ptr);
-    }
-
-    m_lg = m_lg + str.m_lg;
-
-    delete[] m_ptr;
-
-    m_ptr = p;
-
     return *this;
 }
 
 bool CBotString::operator==(const CBotString& str)
 {
-    return Compare(str) == 0;
+    return m_str == str.m_str;
 }
 
 bool CBotString::operator==(const char* p)
 {
-    return Compare(p) == 0;
+    return m_str == p;
 }
 
 bool CBotString::operator!=(const CBotString& str)
 {
-    return Compare(str) != 0;
+    return m_str != str.m_str;
 }
 
 bool CBotString::operator!=(const char* p)
 {
-    return Compare(p) != 0;
+    return m_str != p;
 }
 
 bool CBotString::operator>(const CBotString& str)
 {
-    return Compare(str) > 0;
+    return m_str > str.m_str;
 }
 
 bool CBotString::operator>(const char* p)
 {
-    return Compare(p) > 0;
+    return m_str > p;
 }
 
 bool CBotString::operator>=(const CBotString& str)
 {
-    return Compare(str) >= 0;
+    return m_str >= str.m_str;
 }
 
 bool CBotString::operator>=(const char* p)
 {
-    return Compare(p) >= 0;
+    return m_str >= p;
 }
 
 bool CBotString::operator<(const CBotString& str)
 {
-    return Compare(str) < 0;
+    return m_str < str.m_str;
 }
 
 bool CBotString::operator<(const char* p)
 {
-    return Compare(p) < 0;
+    return m_str < p;
 }
 
 bool CBotString::operator<=(const CBotString& str)
 {
-    return Compare(str) <= 0;
+    return m_str <= str.m_str;
 }
 
 bool CBotString::operator<=(const char* p)
 {
-    return Compare(p) <= 0;
+    return m_str <= p;
 }
 
 bool CBotString::IsEmpty() const
 {
-    return (m_lg == 0);
+    return m_str.empty();
 }
 
 void CBotString::Empty()
 {
-    delete[] m_ptr;
+    m_str.clear();
-    m_ptr = nullptr;
-    m_lg = 0;
 }
 
-static char emptyString[] = {0};
+static char emptyString[] = "";
 
 CBotString::operator const char * () const
 {
-    if (this == nullptr || m_ptr == nullptr) return emptyString;
+    if (this == nullptr) return emptyString; // TODO: can this be removed?
-    return m_ptr;
+    return m_str.c_str();
 }
 
 const char* CBotString::CStr() const
 {
-    if (this == nullptr || m_ptr == nullptr)
+    if (this == nullptr) return emptyString; // TODO: can this be removed?
-    {
+    return m_str.c_str();
-        return emptyString;
-    }
-    return m_ptr;
-}
-
-int CBotString::Compare(const char * lpsz) const
-{
-    char* p = m_ptr;
-    if (lpsz  == nullptr) lpsz = emptyString;
-    if (m_ptr == nullptr) p = emptyString;
-    return strcmp(p, lpsz);    // wcscmp
 }
 
 const char * CBotString::MapIdToString(EID id)

src/CBot/CBotString.h

@@ -31,13 +31,14 @@
 
 /**
  * \brief CBotString Class used to work on strings
+ * TODO: do not use this class, code should be refactored to use std::string instead
  */
 class CBotString
 {
 public:
 
     /**
-     * \brief CBotString Default constructor.
+     * \brief CBotString Creates an empty string
      */
     CBotString();
 
@@ -47,6 +48,12 @@ public:
      */
     CBotString(const char* p);
 
+    /**
+     * \brief CBotString
+     * \param p
+     */
+    CBotString(const std::string& p);
+
     /**
      * \brief CBotString
      * \param p
@@ -145,15 +152,6 @@ public:
      */
     CBotString Right(int nCount) const;
 
-    /**
-     * \brief Compare Compare a given string to an other.
-     * \param lpsz    The string to compare.
-     * \return        0 if the two string matches. Less than 0 if the current
-     *                string is less than lpsz. Greater than 0 if the current
-     *                string is greater than lpsz.
-     */
-    int Compare(const char* lpsz) const;
-
     /**
      * \brief MakeUpper Uppercase the string.
      */
@@ -198,11 +196,8 @@ public:
 
 private:
 
-    //! \brief Pointer to string
+    //! \brief String
-    char* m_ptr;
+    std::string m_str;
-
-    //! \brief Length of the string
-    int m_lg;
 
     //! \brief Keeps the string corresponding to keyword ID
     static const std::map<EID, const char *> s_keywordString;

src/CBot/CBotStringArray.cpp

@@ -17,112 +17,35 @@
  * along with this program. If not, see http://gnu.org/licenses
  */
 
-// Modules inlcude
 #include "CBot/CBotStringArray.h"
 
-#include "CBot/CBotUtils.h"
-
-// Local include
-
-// Global include
-
-// Forward declaration
-#include <cstring>
-
 ////////////////////////////////////////////////////////////////////////////////
 CBotStringArray::CBotStringArray()
 {
-    m_pData = nullptr;
-    m_nSize = m_nMaxSize = 0;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 CBotStringArray::~CBotStringArray()
 {
-    SetSize(0);                    // destroys data !
+    m_data.clear(); // destroys data !
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 int CBotStringArray::GetSize()
 {
-    return m_nSize;
+    return m_data.size();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 void CBotStringArray::Add(const CBotString& str)
 {
-    SetSize(m_nSize+1);
+    m_data.push_back(str);
-
-    m_pData[m_nSize-1] = str;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 void CBotStringArray::SetSize(int nNewSize)
 {
-    if (nNewSize == 0)
+    m_data.resize(nNewSize);
-    {
-        // shrink to nothing
-
-        DestructElements(m_pData, m_nSize);
-        delete[] reinterpret_cast<unsigned char *>(m_pData);
-        m_pData = nullptr;
-        m_nSize = m_nMaxSize = 0;
-    }
-    else if (m_pData == nullptr)
-    {
-        // create one with exact size
-        m_pData = reinterpret_cast<CBotString*> (new unsigned char[nNewSize * sizeof(CBotString)]);
-
-        ConstructElements(m_pData, nNewSize);
-
-        m_nSize = m_nMaxSize = nNewSize;
-    }
-    else if (nNewSize <= m_nMaxSize)
-    {
-        // it fits
-        if (nNewSize > m_nSize)
-        {
-            // initialize the new elements
-
-            ConstructElements(&m_pData[m_nSize], nNewSize-m_nSize);
-
-        }
-
-        else if (m_nSize > nNewSize)  // destroy the old elements
-            DestructElements(&m_pData[nNewSize], m_nSize-nNewSize);
-
-        m_nSize = nNewSize;
-    }
-    else
-    {
-        // otherwise, grow array
-        int nGrowBy;
-        {
-            // heuristically determine growth when nGrowBy == 0
-            //  (this avoids heap fragmentation in many situations)
-            nGrowBy = std::min(1024, std::max(4, m_nSize / 8));
-        }
-        int nNewMax;
-        if (nNewSize < m_nMaxSize + nGrowBy)
-            nNewMax = m_nMaxSize + nGrowBy;  // granularity
-        else
-            nNewMax = nNewSize;  // no slush
-
-        CBotString* pNewData = reinterpret_cast<CBotString*> (new unsigned char[nNewMax * sizeof(CBotString)]);
-
-        // copy new data from old
-        memcpy(pNewData, m_pData, m_nSize * sizeof(CBotString));
-
-        // construct remaining elements
-        ConstructElements(&pNewData[m_nSize], nNewSize-m_nSize);
-
-
-        // Get rid of old stuff (note: no destructors called)
-        delete[] reinterpret_cast<unsigned char *>(m_pData);
-        m_pData = pNewData;
-        m_nSize = nNewSize;
-        m_nMaxSize = nNewMax;
-    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -134,5 +57,5 @@ CBotString& CBotStringArray::operator[](int nIndex)
 ////////////////////////////////////////////////////////////////////////////////
 CBotString& CBotStringArray::ElementAt(int nIndex)
 {
-    return m_pData[nIndex];
+    return m_data[nIndex];
 }

src/CBot/CBotStringArray.h

@@ -19,15 +19,13 @@
 
 #pragma once
 
-// Modules inlcude
 #include "CBot/CBotString.h"
 
-// Local include
+#include <vector>
-
-// Global include
 
 /*!
  * \brief The CBotStringArray class Class used to arrays of strings management.
+ * TODO: refactor code to use std::vector instead
  */
 class CBotStringArray : public CBotString
 {
@@ -77,9 +75,5 @@ public:
 
 private:
 
-    //! Number of elements.
+    std::vector<CBotString> m_data;
-    int m_nSize;
-    //! Reserved size.
-    int m_nMaxSize;
-    CBotString* m_pData;
 };

test/unit/CBot/CBotString_test.cpp

@@ -159,22 +159,6 @@ TEST(CBotString_Test, Right)
     EXPECT_STREQ(str1.Right(7).CStr(), "Colobot");
 }
 
-/**
- * @brief CBotString_Test.Compare Check Compare function of class
- *                                CBotString_Test
- */
-TEST(CBotString_Test, Compare)
-{
-    CBotString str1("colobot");
-    EXPECT_EQ(0, str1.Compare("colobot"));
-
-    CBotString str2("a");
-    EXPECT_GT(0, str2.Compare("colobot"));
-
-    CBotString str3("z");
-    EXPECT_LT(0, str3.Compare("colobot"));
-}
-
 /**
  * @brief CBotString_Test.MakeUpper Check MakeUpper function of class
  *                                  CBotString_Test