22
\$\begingroup\$

I've recreated std::string in a way that might be faster.

System Features:

  • Much faster than std::string.

  • Easy to use.

  • It was designed as part of a large database so it is very fast and good at handling memory.

  • Responsive to all types of data, you can enter characters, numbers, even decimal numbers, boolean values ​​or even lists, without the need to convert data types to a string.

  • It contains a system dedicated to calculating numbers and converting them into text strings, It works great with decimal numbers. see below to learn more.

  • All functions tested, no errors detected.

The reason for the speed of performance:

The reason lies in the way I handle the heap memory, I don't need to realloc memory every time the value changes, and I don't need to completely reset the object when any modification is made.

If you are interested you can check out the set() function, i have left comments that can explain this further.

Test results:

+-------------------------+--------+------+------------------------------+
|METHOD                   |CLASS   |TIME  |NOTES                         |
+-------------------------+--------+------+------------------------------+
|create empty             |xstring |210   |                              |
|                         |string  |1586  |                              |
+-------------------------+--------+------+------------------------------+
|create by string         |xstring |1859  |                              |
|                         |string  |3194  |                              |
+-------------------------+--------+------+------------------------------+
|create by character      |xstring |1852  |                              |
|                         |string  |2680  |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by bool           |xstring |1836  |                              |
|                         |string  |2487  |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by integer        |xstring |2477  |                              |
|                         |string  |2453  |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by decimal        |xstring |4428  |                              |
|                         |string  |23053 |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by same object    |xstring |3750  |                              |
|                         |string  |9779  |                              |
+-------------------------+--------+------+------------------------------+
|create by multiple       |xstring |3726  |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|append                   |xstring |2426  |                              |
|                         |string  |7685  |                              |
+-------------------------+--------+------+------------------------------+
|prepend                  |xstring |2593  |                              |
|                         |string  |10665 |Unavailable, used insert(0,)  |
+-------------------------+--------+------+------------------------------+
|insert                   |xstring |2574  |                              |
|                         |string  |10579 |                              |
+-------------------------+--------+------+------------------------------+
|compare                  |xstring |3985  |                              |
|                         |string  |8200  |                              |
+-------------------------+--------+------+------------------------------+
|swap                     |xstring |3928  |                              |
|                         |string  |11579 |                              |
+-------------------------+--------+------+------------------------------+
|to lower                 |xstring |2407  |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|to upper                 |xstring |2432  |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|select by index          |xstring |1984  |                              |
|                         |string  |3303  |                              |
+-------------------------+--------+------+------------------------------+
|select by char and pos   |xstring |1976  |                              |
|                         |string  |4138  |                              |
+-------------------------+--------+------+------------------------------+
|select last              |xstring |1910  |                              |
|                         |string  |3563  |                              |
+-------------------------+--------+------+------------------------------+
|pop last                 |xstring |2114  |                              |
|                         |string  |4338  |                              |
+-------------------------+--------+------+------------------------------+
|pop by index             |xstring |2095  |                              |
|                         |string  |6591  |                              |
+-------------------------+--------+------+------------------------------+
|reverse                  |xstring |2465  |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|find first of            |xstring |2001  |                              |
|                         |string  |4446  |                              |
+-------------------------+--------+------+------------------------------+
|find last of             |xstring |1979  |                              |
|                         |string  |4199  |                              |
+-------------------------+--------+------+------------------------------+
|sub string               |xstring |4070  |                              |
|                         |string  |17287 |                              |
+-------------------------+--------+------+------------------------------+
|is empty                 |xstring |1909  |                              |
|                         |string  |3262  |                              |
+-------------------------+--------+------+------------------------------+

Test Info

  • OS linux, ubuntu 20.04

  • processor Intel(R) Core(TM) i7-6500U CPU @ 2.50GHz

  • memory 8064MiB System memory

  • Compiler g++ 11

  • C++ Version c++ 20

  • tested by valgrind, you can test with just run script and you will got same ratios

You can find the source file and test script here GitHub: xString

I have some questions :

  1. Are there any errors that I did not notice?

  2. Is there anything that can be done to improve performance?

  3. Is there anything missing?

  4. What is your opinion and what are your suggestions?


Edit: xstring.h

/*
    All rights reserved: Maysara Khalid Elshewehy [github.com/xeerx]
    Created on: 4 Apr 2022
    Commercial use of this file is prohibited without written permission
*/

#ifndef XCPP_XSTRING_H
#define XCPP_XSTRING_H
#include <string.h>                // strlen
#include <cstdlib>                 // calloc, realloc, free
#include <initializer_list>        // initializer_list
#include <stddef.h>                // size_t
#include <stdint.h>                // SIZE_MAX
#include <ostream>                 // ostream operator

// xstring macros
#define xnpos            SIZE_MAX  // max size of size_t, when error, return to it
#define _xs_extra_space_ 25        // the extra space size in heap memory

// xstring helpers
namespace x::hlpr
{
    // [INTEGER LENGTH]
    #define __sig_int_len__(t) (t int src)                   { unsigned t int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    #define __uns_int_len__(t) (unsigned t int src)          { unsigned t int len = 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    unsigned int intlen __sig_int_len__() 
    unsigned int uintlen __uns_int_len__() 
    unsigned short int sintlen __sig_int_len__(short) 
    unsigned short int usintlen __uns_int_len__(short) 
    unsigned long lintlen __sig_int_len__(long) 
    unsigned long ulintlen __uns_int_len__(long) 
    unsigned long long llintlen __sig_int_len__(long long) 
    unsigned long long ullintlen __uns_int_len__(long long)

    // [INTEGER TO STRING]
    #define __sig_int_str__(s,t) (char *ref, s src, unsigned s len = 0) { if(src == 0){ ref[0] = '0'; ref[1] = '\0'; return; } if (len == 0) len = t(src) + 1; if (src < 0) { src = src * -1; ref[0] = '-'; } len--; ref[len + 1] = '\0'; while (src > 0) { ref[len] = src % 10 + '0'; src /= 10; len--; } } 
    #define __uns_int_str__(s,t) (char *ref, unsigned s src, unsigned s len = 0) { if(src == 0){ ref[0] = '0'; ref[1] = '\0'; return; } if(len == 0) len = t(src) +1; len --; ref[len+1] = '\0'; while(src >= 1) { ref[len] = src % 10 + '0'; src /= 10; len --; } }
    
    void intstr    __sig_int_str__(int,intlen) 
    void uintstr   __uns_int_str__(int ,uintlen)
  
    void sintstr   __sig_int_str__(short int,sintlen) 
    void usintstr  __uns_int_str__(short int ,usintlen)

    void lintstr   __sig_int_str__(long int,lintlen)
    void ulintstr  __uns_int_str__(long int ,ulintlen)

    void llintstr  __sig_int_str__(long long int,llintlen) 
    void ullintstr __uns_int_str__(long long int ,ullintlen)

    // [INTEGER MACROS]
    #define __int_str_shortcut__(src,l,lfunc,sfunc)  unsigned l __len = x::hlpr::lfunc(src) +1; char * __ref = new char[__len+1]; x::hlpr::sfunc(__ref,src,__len); delete [] __ref;
    
    #define __intstr__(src)    __int_str_shortcut__(src,int,intlen,intstr)
    #define __uintstr__(src)   __int_str_shortcut__(src,int,uintlen,uintstr)

    #define __sintstr__(src)   __int_str_shortcut__(src,short int,sintlen,sintstr)
    #define __usintstr__(src)  __int_str_shortcut__(src,short int,usintlen,usintstr)
    
    #define __lintstr__(src)   __int_str_shortcut__(src,long int,lintlen,lintstr)
    #define __ulintstr__(src)  __int_str_shortcut__(src,long int,ulintlen,ulintstr)

    #define __llintstr__(src)  __int_str_shortcut__(src,long long int,llintlen,llintstr)
    #define __ullintstr__(src) __int_str_shortcut__(src,long long int,ullintlen,ullintstr)

    // [DECIMAL LENGTH]
    #define __decimal_len__(t) (t src, unsigned short int max = 6) { unsigned int len = 0, ilen = 0, i = 0; int isrc = (int)src; if(src < 0) src *= -1; if(src > 0) { ilen = intlen(isrc); src -= isrc;}  while(i < max && (isrc - src) != 0) { src = src * 10; i++; len ++; } return len + ilen; }
    unsigned int floatlen   __decimal_len__(float)
    unsigned int doublelen  __decimal_len__(double)
    unsigned int ldoublelen __decimal_len__(long double)
    
    // [DECIMAL TO STRING]
    #define __decimal_str__(t,lfunc) (char *ref, t src, unsigned short int max = 0, bool setmax = false) { bool neg = false; if(src < 0) { src = src * -1; neg = true; } if(src == 0) { ref[0] = '0'; ref[1] = '.'; ref[2] = '0'; ref[3] = '\0'; } int numbers = src, digit = 0, numberslen = x::hlpr::intlen(numbers), index = numberslen + 1; long pow_value = 0; unsigned int flen = x::hlpr::lfunc(src); if(numberslen > 0) flen -= numberslen; if(!max) max = flen; else if(setmax) max -= numberslen; max --; x::hlpr::intstr(ref,numbers,index); ref[index] = '.'; index ++; if(numbers == src) { ref[index] = '0'; index ++; if(neg) { unsigned int bi = index; ref[bi] = '\0'; while (index >= 0) { ref[index+1] = ref[index]; index --; } ref[0] = '-'; return bi +1; } else ref[index] = '\0'; return index; }  t fraction_part  = src - numbers; t tmp1 = fraction_part,  tmp =0; for( int i= 1; i < max + 2; i++) { pow_value = 10; tmp = tmp1 * pow_value; digit = tmp; ref[index] = digit +48; tmp1 = tmp - digit; index ++; } int temp_index = index -1; while(temp_index >= 0) { if(ref[temp_index] == '0') { ref[temp_index] = '\0'; index --;  } else break; temp_index --; } temp_index = index -1; while(temp_index >= 0) { if(ref[temp_index] == ref[temp_index-1]) { ref[temp_index] = '\0'; index --;  } else break; temp_index --; } if(neg) { unsigned int bi = index; ref[bi] = '\0'; while (index >= 0) { ref[index+1] = ref[index]; index --; } ref[0] = '-'; return bi+1; } else ref[index] = '\0'; return index;}
    unsigned int  floatstr   __decimal_str__(float,floatlen)
    unsigned int  doublestr  __decimal_str__(double,doublelen)
    unsigned int  ldoublestr __decimal_str__(long double,ldoublelen)

    // [DECIMAL MACROS]
    #define __decimal_str_shortcut__(src,lfunc,sfunc,max)  unsigned short int __len = x::hlpr::lfunc(src,max) +1; char * __ref = new char[__len+4];__len = x::hlpr::sfunc(__ref,src,__len,true); delete [] __ref;
    #define __floatstr__(src)   __decimal_str_shortcut__(src,floatlen,floatstr,max)
    #define __doublestr__(src)  __decimal_str_shortcut__(src,doublelen,doublestr,max)
    #define __ldoublestr__(src) __decimal_str_shortcut__(src,ldoublelen,ldoublestr,max)
}

// xstring class
namespace x
{
    // some variables used to return to error
    char xstr_null_char = '\0';

    class xstring
    {
        public:
        
        // data
        char    *src  = nullptr;   // pointer to character array wich contains to value of object
        size_t   len  = 0;         // length of src, zero means empty
        size_t   siz  = 0;         // size of src in heap memory

        // constructor -> default empty
        xstring         ()                                       {                                                                  }

        xstring         (const char *ref, size_t rlen = 0)       { set(ref,rlen);                                                   }
        xstring         (char        ref)                        { set(ref);                                                        }

        xstring         (bool        ref)                        { set(ref);                                                        }

        xstring         (                   int ref)             { set(ref);                                                        }
        xstring         (unsigned           int ref)             { set(ref);                                                        }

        xstring         (short              int ref)             { set(ref);                                                        }
        xstring         (unsigned short     int ref)             { set(ref);                                                        }

        xstring         (long               int ref)             { set(ref);                                                        }
        xstring         (unsigned long      int ref)             { set(ref);                                                        }
        
        xstring         (long long          int ref)             { set(ref);                                                        }
        xstring         (unsigned long long int ref)             { set(ref);                                                        }

        xstring         (float       ref,unsigned short max = 1) { set(ref,max);                                                    }
        xstring         (double      ref,unsigned short max = 1) { set(ref,max);                                                    }
        xstring         (long double ref,unsigned short max = 1) { set(ref,max);                                                    }

        xstring         (xstring    &ref)                        { set(ref);                                                        }
        xstring         (const xstring    &ref)                  { set(ref);                                                        }

        template <typename T>
        xstring         (std::initializer_list<T> ref)           { for(auto i:ref) append(i);                                       }

        // set
        bool set        (const char *ref, size_t rlen);
        bool set        (char        ref)                        { char __ref[] = {ref}; return set(__ref, 1);                      }

        bool set        (bool        ref)                        { return set(ref ? "1" : "0", 1);                                  }

        bool set        (                   int ref)             { __intstr__     (ref) return set(__ref, __len);                   }
        bool set        (unsigned           int ref)             { __uintstr__    (ref) return set(__ref, __len);                   }

        bool set        (short              int ref)             { __sintstr__    (ref) return set(__ref, __len);                   }
        bool set        (unsigned short     int ref)             { __usintstr__   (ref) return set(__ref, __len);                   }

        bool set        (long               int ref)             { __lintstr__    (ref) return set(__ref, __len);                   }
        bool set        (unsigned long      int ref)             { __ulintstr__   (ref) return set(__ref, __len);                   }

        bool set        (long long          int ref)             { __llintstr__   (ref) return set(__ref, __len);                   }
        bool set        (unsigned long long int ref)             { __ullintstr__  (ref) return set(__ref, __len);                   }

        bool set        (float       ref,unsigned short max = 1) { __floatstr__   (ref) return set(__ref, __len);                   }
        bool set        (double      ref,unsigned short max = 1) { __doublestr__  (ref) return set(__ref, __len);                   }
        bool set        (long double ref,unsigned short max = 1) { __ldoublestr__ (ref) return set(__ref, __len);                   }
        
        bool set        (xstring    &ref)                        { if(&ref != this) return set(ref.src, ref.len); else return false;  }
        bool set        (const xstring    &ref)                  { if(&ref != this) return set(ref.src, ref.len); else return false;  }

        template <typename T>
        bool set        (std::initializer_list<T> ref)           { reset(); for(auto i:ref) if(!append(i)) return false; return true; }

        template <typename T> 
        bool operator = (T ref) { return set(ref); }

        template <typename T> 
        bool operator = (std::initializer_list<T> ref) { return set(ref); }

        // append
        bool append  (const char *ref, size_t rlen);
        bool append  (char        ref)                        { char __ref[] = {ref}; return append(__ref, 1);                      }

        bool append  (bool        ref)                        { return append(ref ? "1" : "0", 1);                                  }

        bool append  (                   int ref)             { __intstr__     (ref) return append(__ref, __len);                   }
        bool append  (unsigned           int ref)             { __uintstr__    (ref) return append(__ref, __len);                   }

        bool append  (short              int ref)             { __sintstr__    (ref) return append(__ref, __len);                   }
        bool append  (unsigned short     int ref)             { __usintstr__   (ref) return append(__ref, __len);                   }

        bool append  (long               int ref)             { __lintstr__    (ref) return append(__ref, __len);                   }
        bool append  (unsigned long      int ref)             { __ulintstr__   (ref) return append(__ref, __len);                   }

        bool append  (long long          int ref)             { __llintstr__   (ref) return append(__ref, __len);                   }
        bool append  (unsigned long long int ref)             { __ullintstr__  (ref) return append(__ref, __len);                   }

        bool append  (float       ref,unsigned short max = 1) { __floatstr__   (ref) return append(__ref, __len);                   }
        bool append  (double      ref,unsigned short max = 1) { __doublestr__  (ref) return append(__ref, __len);                   }
        bool append  (long double ref,unsigned short max = 1) { __ldoublestr__ (ref) return append(__ref, __len);                   }
        
        bool append  (xstring    &ref)                        { if(&ref != this) return append(ref.src, ref.len); else return false;  }
        bool append  (const xstring    &ref)                  { if(&ref != this) return append(ref.src, ref.len); else return false;  }

        template <typename T>
        bool append  (std::initializer_list<T> ref)           { for(auto i:ref) if(!append(i)) return false; return true;           }

        template <typename T> 
        bool operator += (T ref)                              { return append(ref);                                                 }

        template <typename T> 
        bool operator += (std::initializer_list<T> ref)       { return append(ref);                                                 }
        
        // prepend
        bool prepend    (const char *ref, size_t rlen);
        bool prepend    (char        ref)                        { char __ref[] = {ref}; return prepend(__ref, 1);                  }

        bool prepend    (bool        ref)                        { return prepend(ref ? "1" : "0", 1);                              }

        bool prepend    (                   int ref)             { __intstr__     (ref) return prepend(__ref, __len);               }
        bool prepend    (unsigned           int ref)             { __uintstr__    (ref) return prepend(__ref, __len);               }

        bool prepend    (short              int ref)             { __sintstr__    (ref) return prepend(__ref, __len);               }
        bool prepend    (unsigned short     int ref)             { __usintstr__   (ref) return prepend(__ref, __len);               }

        bool prepend    (long               int ref)             { __lintstr__    (ref) return prepend(__ref, __len);               }
        bool prepend    (unsigned long      int ref)             { __ulintstr__   (ref) return prepend(__ref, __len);               }

        bool prepend    (long long          int ref)             { __llintstr__   (ref) return prepend(__ref, __len);               }
        bool prepend    (unsigned long long int ref)             { __ullintstr__  (ref) return prepend(__ref, __len);               }

        bool prepend    (float       ref,unsigned short max = 1) { __floatstr__   (ref) return prepend(__ref, __len);               }
        bool prepend    (double      ref,unsigned short max = 1) { __doublestr__  (ref) return prepend(__ref, __len);               }
        bool prepend    (long double ref,unsigned short max = 1) { __ldoublestr__ (ref) return prepend(__ref, __len);               }
        
        bool prepend    (xstring    &ref)                        { if(&ref != this) return prepend(ref.src, ref.len); else return false;  }
        bool prepend    (const xstring    &ref)                  { if(&ref != this) return prepend(ref.src, ref.len); else return false;  }

        template <typename T>
        bool prepend    (std::initializer_list<T> ref)           { for(auto i:ref) if(!prepend(i)) return false; return true;       }

        template <typename T> 
        bool operator |= (T ref)                                 { return prepend(ref);                                             }

        template <typename T> 
        bool operator |= (std::initializer_list<T> ref)          { return prepend(ref);                                             }

        // print
        friend std::ostream &operator<<(std::ostream &os, const xstring &s) { os << (s.src ? s.src : ""); return (os); }

        // insert
        bool          insert (size_t pos, const char *ref, size_t rlen);
        bool          insert (size_t pos, char ref)              { char __ref[] = {ref}; return insert(pos,__ref, 1);               }

        // compare between two strings
        bool compare(const char *ref) { return strcmp(src,ref)     == 0;                                                            }
        bool compare(xstring&    ref) { return compare(ref.src);                                                                    }
        bool compare(const xstring&    ref) { return compare(ref.src);                                                              }
        bool compare(      char  ref) { char __ref[] = {ref}; return compare(__ref);                                                }
        bool operator == (const char *ref)          { return compare(ref) == true;                                                  }
        bool operator == (xstring&    ref)          { return compare(ref) == true;                                                  }
        bool operator == (const xstring&    ref)    { return compare(ref) == true;                                                  }
        bool operator == (      char  ref)          { return compare(ref) == true;                                                  }
        
        // no need in C++ 20
        // bool operator != (const char *ref)          { return compare(ref) == false;                                                 }
        // bool operator != (xstring&    ref)          { return compare(ref) == false;                                                 }
        // bool operator != (const xstring&    ref)    { return compare(ref) == false;                                                 }
        // bool operator != (      char  ref)          { return compare(ref) == false;                                                 }

        // swap value,len,size between two string objects
        void swap(xstring &ref)
        {
            char     *tmp_src  = src;
            size_t    tmp_len  = len;
            size_t    tmp_siz  = siz;
 
            src  = ref.src;
            len  = ref.len;
            siz  = ref.siz;

            ref.src  = tmp_src;
            ref.len  = tmp_len;
            ref.siz  = tmp_siz;
        }

        // convert src characters to lower case
        void to_lower() 
        {
            size_t i = 0;
            while(src[i] != '\0') { src[i] = std::tolower(src[i]); i++; }
        }

        // convert src characters to upper case
        void to_upper() 
        {
            size_t i = 0;
            while(src[i] != '\0') { src[i] = std::toupper(src[i]); i++; }
        }

         // select character by index, return xstr_null_char if not found
        char& getChar(size_t ref) 
        {
            if(ref > len) return xstr_null_char;
            return src[ref];
        } 
        char& operator[](size_t ref) { return getChar(ref); }

        // select index by value, start from position, return xnpos if not found
        size_t getIndex(char ref, size_t pos = 0) 
        {
            if(pos > len) return xnpos;
            size_t counter = pos;
            while(counter < len) 
            {
                if(src[counter] == ref) return counter;
                counter ++;
            }
            return xnpos;
        }

        // get last character
        char& getLast() { return src[len-1]; }
        
        // remove from value by index
        bool pop(size_t pos = 0, size_t max = 1)
        {
            if(pos >= len) return false;
            if(max > len || max == 0) max = len;

            memmove(&src[pos], &src[pos + max], len - pos - 1);
            src[len - 1] = '\0';
            len --;
            return true;
        }

        // remove last character using pop(len-1)
        bool pop_last() { return pop(len-1); }

        // reverse characters
        void reverse()
        {
            char *tmp = new char[len];
            size_t count = len;
            size_t count_src = 0;
            while(count > 0)
            {
                tmp[count-1] = src[count_src];
                count --;
                count_src ++;
            }
            tmp[len] = '\0';
            memcpy(src,tmp,len+1);
            delete [] tmp;
        }

        // check if value is empty by check if length equals zero
        bool empty() { return len == 0; }

        // find first matched character in string, start from begin + pos, if failed return to xnpos
        size_t first_of(char  ref, size_t pos = 0)
        {
            if(pos >= len) return xnpos;
            while( src[pos]!=ref && pos < len) pos++;
            return pos == len ? xnpos : pos;
        }

        // find last matched character in string, start from end - pos, if failed return to xnpos
        size_t last_of (char  ref, size_t pos = 0)
        {
            if(pos >= len) return xnpos;
            pos = len - pos -1; 
            while(src[pos]!=ref && pos > 0) pos--;
            if(src[pos] == ref) return pos; else return xnpos;
        }

        // cut string from [start] to [end] and set to ref
        bool sub(xstring &ref, size_t start = 0, size_t end = 0)
        {
            if(start >= len) start = len -1;
            if(start == end) return false;

            if(end == 0 || end >= len) end = len;
            size_t llen = end - start;
            char *res = new char[llen+1];
            size_t pos = 0;

            while(start < end) 
            {
                res[pos] = src[start];
                start ++;
                pos ++;
            }
            res[pos] = '\0';
            ref.set(res, llen+1);
            delete [] res;
            return true;
        }

        // free memory, set values to zero
        void reset      ()                                       { if(!siz) return; free(src); len = 0; siz = 0;                    }
        
        // destructor
        ~xstring        ()                                       { reset();                                                         }
    };
}

// xstring class functions, i will write long functions here to arrange the file and keep it beautiful
namespace x
{
    /**
     * @brief set value of xstring object
    */
    bool xstring::set        (const char *ref, size_t rlen = 0)
    {
        if(!rlen) rlen = strlen(ref);                // [increase performance] If the length is known, there is no need to recalculate it

        if(siz == 0)                                 // alloc space in heap memory to store value
        {
            siz = rlen + _xs_extra_space_;           // [increase performance] extra 25 bytes space to avoid realloc in next times if space is enough
            if(siz >= xnpos - 25) { siz -= rlen + _xs_extra_space_; return false; } // check if size > max size
            src = (char*) calloc(siz, sizeof(char)); // allocate space
            if(src == nullptr) return false;         // check if space is allocated
        }

        else if(siz <= rlen)                         // realloc more space to contains the new value
        {
            siz = rlen + _xs_extra_space_;     
            if(siz >= xnpos - 25) { siz -= rlen + _xs_extra_space_; return false; }
            src = (char*) realloc(src, siz * sizeof(char)); 
            if(src == nullptr) return false; 
        }

        memcpy(src, ref, rlen+1);                    // copy value from ref
        len = rlen;                                  // update length
        src[len] = '\0';                             // terminate the array
        return true;
    }

    /**
     * @brief add value to begin of xstring object
    */
    bool xstring::append  (const char *ref, size_t rlen = 0)
    {
        if(siz == 0) return set(ref,rlen); // if space is null, call set()
        if(!rlen) rlen = strlen(ref);      

        if(siz <= (len + rlen))            
        {
            siz += rlen + _xs_extra_space_;
            if(siz >= xnpos - 25) { siz -= rlen + _xs_extra_space_; return false; }
            src = (char*) realloc(src, siz * sizeof(char)); 
            if(src == nullptr) return false;
        }

        memcpy(src + len, ref, rlen+1);
        len += rlen; 
        src[len] = '\0'; 
        return true;
    }

    /**
     * @brief add value to end of xstring object
    */
    bool xstring::prepend (const char *ref, size_t rlen = 0)
    {
        if(siz == 0) return set(ref,rlen);
        if(!rlen) rlen = strlen(ref);

        if(siz <= (len + rlen))
        {
            siz += rlen + _xs_extra_space_;
            if(siz >= xnpos - 25) { siz -= rlen + _xs_extra_space_; return false; }
            src = (char*) realloc(src, siz * sizeof(char)); if(src == nullptr) return false;
        }

        
        memmove(src + rlen, src, len);      // move data to rlen pos
        size_t i = 0; 
        while(i < rlen)                     // loop in copy ref to begin of src
        { 
            src[i] = ref[i]; 
            ++i; 
        }
        len += rlen; 
        src[len] = '\0'; 
        return true;
    }
    
    /**
     * @brief add value to position of xstring object
    */
    bool xstring::insert (size_t pos, const char *ref, size_t rlen = 0)
    {
        if(siz == 0) return set(ref,rlen);
        if(pos > len) return false;
        if(!rlen) rlen = strlen(ref);

        if(siz <= (len + rlen))
        {
            siz += rlen + _xs_extra_space_;
            if(siz >= xnpos - 25) { siz -= rlen + _xs_extra_space_; return false; }
            src = (char*) realloc(src, siz * sizeof(char)); if(src == nullptr) return false;
        }

        memmove(src + rlen, src, len);
        size_t i = 0; while(true) { size_t target = pos == 0 ? (0 + i) : (pos+i); src[target] = ref[i]; if(++i >= rlen) break;; }
        len += rlen; src[len] = '\0'; return true;
    }

    
}

#endif

test.cpp

#include "xstring.h"
#include <chrono>
#include <string>
#include <iostream>
#include <iomanip>
#include <limits.h>
#include <float.h>

using namespace x;
using namespace std;
void    TEST_PRNT (const char*f, const char*t, const char*r, const char*n, bool row = false) { char sep = row ? '+' : '|'; std::cout << std::left << sep << std::setw(25) << f << sep << std::setw(8) << t << sep  << std::setw(6) << r  << sep << std::setw(30) <<  n << sep << std::endl; }
#define TEST_STUP std::chrono::steady_clock::time_point test_begin; std::chrono::steady_clock::time_point test_end; char test_result[75]; TEST_PRNT("-------------------------", "--------", "------", "------------------------------",true);TEST_PRNT("METHOD", "CLASS", "TIME", "NOTES");TEST_PRNT("-------------------------", "--------", "------", "------------------------------",true);
#define TEST_STRT test_begin = std::chrono::steady_clock::now(); for(int i = 0; i < 1000000; ++i) {
#define TEST_STOP } test_end = std::chrono::steady_clock::now(); sprintf(test_result,"%-6ld",(std::chrono::duration_cast<std::chrono::microseconds>(test_end - test_begin).count()) / 1000);
#define TEST_LINE TEST_PRNT("-------------------------", "--------", "------", "------------------------------", true);

using namespace x;


int main()
{
    std::cout << "----------------------------------------" << std::endl;
    std::cout << "USE: g++ --std c++20 -g test.cpp -o test" << std::endl;
    std::cout << "----------------------------------------" << std::endl;

    TEST_STUP

    // create empty
    TEST_STRT xstring x; TEST_STOP TEST_PRNT("create empty","xstring"  ,test_result,""); 
    TEST_STRT string  x; TEST_STOP TEST_PRNT("            ","string" ,test_result,""); TEST_LINE

    // create by string
    TEST_STRT xstring x = "value"; TEST_STOP TEST_PRNT("create by string","xstring"  ,test_result,""); 
    TEST_STRT string  x = "value"; TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // create by character
    TEST_STRT xstring x = 'c';            TEST_STOP TEST_PRNT("create by character","xstring"  ,test_result,""); 
    TEST_STRT string  x = to_string('c'); TEST_STOP TEST_PRNT("                   ","string" ,test_result,"Unavailable, used to_string()"); TEST_LINE

    // create by bool
    TEST_STRT xstring x = true;            TEST_STOP TEST_PRNT("create by bool","xstring"  ,test_result,""); 
    TEST_STRT string  x = to_string(true); TEST_STOP TEST_PRNT("              ","string" ,test_result,"Unavailable, used to_string()"); TEST_LINE

    // create by integer
    TEST_STRT xstring x = 1;            TEST_STOP TEST_PRNT("create by integer","xstring"  ,test_result,""); 
    TEST_STRT string  x = to_string(1); TEST_STOP TEST_PRNT("                 ","string" ,test_result,"Unavailable, used to_string()"); TEST_LINE

    // create by decimal
    TEST_STRT xstring x = 123.456;            TEST_STOP TEST_PRNT("create by decimal","xstring"  ,test_result,""); 
    TEST_STRT string  x = to_string(123.456); TEST_STOP TEST_PRNT("                 ","string" ,test_result,"Unavailable, used to_string()"); TEST_LINE

    // create by same object
    TEST_STRT xstring y = "val"; xstring x = y;  TEST_STOP TEST_PRNT("create by same object","xstring"  ,test_result,""); 
    TEST_STRT string  y = "val"; string  x = y;  TEST_STOP TEST_PRNT("                     ","string" ,test_result,""); TEST_LINE

    // create by multiple
    TEST_STRT xstring x = {"Hello"," ","World","!"};  TEST_STOP TEST_PRNT("create by multiple","xstring"  ,test_result,""); 
                                                                TEST_PRNT("                  ","string" ,"--"         ,"Unavailable"); TEST_LINE
    
    // append
    TEST_STRT xstring x = "Hello"; x += "World"; TEST_STOP TEST_PRNT("append","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x += "World"; TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // prepend
    TEST_STRT xstring x = "Hello"; x |= "Hello"; TEST_STOP TEST_PRNT("prepend","xstring"  ,test_result,""); 
    TEST_STRT string  x = "World"; x.insert(0,"Hello"); TEST_STOP TEST_PRNT("             ","string" ,test_result,"Unavailable, used insert(0,)"); TEST_LINE
    
    // insert
    TEST_STRT xstring x = "Hello"; x.insert(3,"Hello"); TEST_STOP TEST_PRNT("insert","xstring"  ,test_result,""); 
    TEST_STRT string  x = "World"; x.insert(3,"Hello"); TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // compare
    TEST_STRT xstring x = "Hello", y = "World"; x.compare(y); TEST_STOP TEST_PRNT("compare"      ,"xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello", y = "World"; x.compare(y); TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // swap
    TEST_STRT xstring x = "Hello", y = "World"; x.swap(y); TEST_STOP TEST_PRNT("swap"      ,"xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello", y = "World"; x.swap(y); TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // to lower
    TEST_STRT xstring x = "Hello"; x.to_lower(); TEST_STOP TEST_PRNT("to lower"      ,"xstring"  ,test_result,""); 
                                                           TEST_PRNT("             ","string" ,"--","Unavailable"); TEST_LINE

    // to upper
    TEST_STRT xstring x = "Hello"; x.to_upper(); TEST_STOP TEST_PRNT("to upper"      ,"xstring"  ,test_result,""); 
                                                           TEST_PRNT("             ","string" ,"--","Unavailable"); TEST_LINE
    
    // select by index
    TEST_STRT xstring x = "Hello"; x[0] = 'x'; TEST_STOP TEST_PRNT("select by index","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x[0] = 'x';  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // select char
    TEST_STRT xstring x = "Hello"; x.getIndex('l',1); TEST_STOP TEST_PRNT("select by char and pos","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.find('l',1);  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // select last
    TEST_STRT xstring x = "Hello"; x.getLast(); TEST_STOP TEST_PRNT("select last","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.back();  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // pop last
    TEST_STRT xstring x = "Hello"; x.pop_last(); TEST_STOP TEST_PRNT("pop last","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.pop_back();  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // pop by index
    TEST_STRT xstring x = "Hello"; x.pop(2,1); TEST_STOP TEST_PRNT("pop by index","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.erase(2,1);  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // reverse
    TEST_STRT xstring x = "Hello"; x.reverse(); TEST_STOP TEST_PRNT("reverse","xstring"  ,test_result,""); 
                                                          TEST_PRNT("             ","string" ,"--","Unavailable"); TEST_LINE

    // find first of
    TEST_STRT xstring x = "Hello"; x.first_of('l'); TEST_STOP TEST_PRNT("find first of","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.find_first_of('l');  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // find last of
    TEST_STRT xstring x = "Hello"; x.last_of('l'); TEST_STOP TEST_PRNT("find last of","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; x.find_last_of('l');  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // sub string
    TEST_STRT xstring x = "Hello",y; x.sub(y, 0,3); TEST_STOP TEST_PRNT("sub string","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello",y; y = x.substr(0,3);  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

    // is empty
    TEST_STRT xstring x = "Hello"; if(x.empty()){} TEST_STOP TEST_PRNT("is empty","xstring"  ,test_result,""); 
    TEST_STRT string  x = "Hello"; if(x.empty()){}  TEST_STOP TEST_PRNT("             ","string" ,test_result,""); TEST_LINE

}

Final Note:

I only started learning C++ a couple of months ago, so I don't have much experience, I wasn't aware of the optimization options, so it seems like I was too quick to judge my project and my abilities!

I will learn more and re-work on the same project to fix my mistakes not to do something faster! I will be glad for your suggestions, and please tell me how to improve the code, what should I do? What do I learn?

The Second Final Note :D

I will stop working for a while, I will try to develop my skills and increase my knowledge of the language and the compiler.

I will not delete the code now, my mistake does not mean my end, on the contrary, I will learn from this mistake and will come back soon with something strong.

\$\endgroup\$
1
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$
    – Mast
    Commented Apr 12, 2022 at 10:10

6 Answers 6

75
\$\begingroup\$

Unfortunately, your implementation is slower (sometimes substantially slower) than std::string according to your own benchmarks in every test but "create by decimal":

+-------------------------+--------+------+------------------------------+
|METHOD                   |CLASS   |TIME  |NOTES                         |
+-------------------------+--------+------+------------------------------+
|create empty             |xstring |0     |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|create by string         |xstring |56    |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|create by character      |xstring |58    |                              |
|                         |string  |7     |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by bool           |xstring |45    |                              |
|                         |string  |4     |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by integer        |xstring |67    |                              |
|                         |string  |5     |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by decimal        |xstring |133   |                              |
|                         |string  |1164  |Unavailable, used to_string() |
+-------------------------+--------+------+------------------------------+
|create by same object    |xstring |555   |                              |
|                         |string  |18    |                              |
+-------------------------+--------+------+------------------------------+
|create by multiple       |xstring |147   |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|append                   |xstring |97    |                              |
|                         |string  |11    |                              |
+-------------------------+--------+------+------------------------------+
|prepend                  |xstring |106   |                              |
|                         |string  |27    |Unavailable, used insert(0,)  |
+-------------------------+--------+------+------------------------------+
|insert                   |xstring |105   |                              |
|                         |string  |27    |                              |
+-------------------------+--------+------+------------------------------+
|compare                  |xstring |154   |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|swap                     |xstring |419   |                              |
|                         |string  |34    |                              |
+-------------------------+--------+------+------------------------------+
|to lower                 |xstring |110   |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|to upper                 |xstring |100   |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|select by index          |xstring |62    |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|select by char and pos   |xstring |64    |                              |
|                         |string  |10    |                              |
+-------------------------+--------+------+------------------------------+
|select last              |xstring |66    |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|pop last                 |xstring |52    |                              |
|                         |string  |5     |                              |
+-------------------------+--------+------+------------------------------+
|pop by index             |xstring |370   |                              |
|                         |string  |18    |                              |
+-------------------------+--------+------+------------------------------+
|reverse                  |xstring |89    |                              |
|                         |string  |--    |Unavailable                   |
+-------------------------+--------+------+------------------------------+
|find first of            |xstring |54    |                              |
|                         |string  |10    |                              |
+-------------------------+--------+------+------------------------------+
|find last of             |xstring |54    |                              |
|                         |string  |5     |                              |
+-------------------------+--------+------+------------------------------+
|sub string               |xstring |139   |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+
|is empty                 |xstring |53    |                              |
|                         |string  |0     |                              |
+-------------------------+--------+------+------------------------------+

See for yourself

Now, I've compiled it with full optimizations enabled (-O2), rather than without optimizations enabled, which is what your "USE" suggestion shows. But it makes no sense to compare the performance of code when not compiling with optimizations enabled.

To me, these test results are a deal-breaker. There's no reason to review this code any further. It doesn't matter how well-written your code is, how nice your comments are, how nicely things are aligned into columns, how well thought-out your interface is, or any of that. The code fails to meet its primary design goal, which is to be faster than the built-in string class. Unless it's faster than the built-in string class, or can achieve things that the built-in string class cannot, then there's no reason for this code to even exist. Writing it, testing it, learning its interface, and everything else is merely a waste of time, for both the author and the consumers.

If string formatting (which is apparently the function that the "create" tests are measuring) is slow, and you want to improve that, then you can improve just that, without any need to introduce another incompatible string class. … But, before you embark on that project, check out C++20's std::format.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ Interestingly enough, icx and clang with -O3 and 2020 C++ standard are much faster for both versions, while still your concern stays true where the difference is measurable. \$\endgroup\$ Commented Apr 9, 2022 at 13:01
  • 21
    \$\begingroup\$ Was morbidly curious why the floating point conversion was faster than the standard. Hypotheses: 1.) It doesn’t do nearly as much; 2.) It’s broken. Turns out the answer is 2. (And, also probably 1.) \$\endgroup\$
    – indi
    Commented Apr 9, 2022 at 16:41
  • 6
    \$\begingroup\$ @JanDorniak Honestly didn’t read through the code much either, because the numeric conversion code is an absolute godforsaken mess of some of the ugliest macro code I’ve seen in years (in C++20 code!), and the floating point conversion macro itself is literally almost 1,500 characters long. But I can say with confidence that, no, it is not locale-aware. It doesn’t even seem to use anything like modf(); it’s all raw arithmetic. I might do a forensic analysis of it, to figure out exactly what went wrong. \$\endgroup\$
    – indi
    Commented Apr 10, 2022 at 1:34
  • 1
    \$\begingroup\$ @xeerx the main reason std::string is efficient is due to short-string-optimization. Dynamic/heap allocations are expensive, so for short strings std::string stores them locally. It is achieved by either memory reporposing, or adding an extra buffer, or both. \$\endgroup\$
    – ALX23z
    Commented Apr 10, 2022 at 6:54
  • 3
    \$\begingroup\$ There is also the distinct possibility that the constructed microbenchmarks are incorrect, and thus that the optimizer is able to elide many of the operations involving std::string. I did not review the code carefully enough to see if that was true. Writing good microbenchmarks is hard, and it takes a fair amount of knowledge of how compilers generate code. If even the best-case benchmark doesn't show a performance win, it's hard to justify digging further, either into improving the code or into improving the accuracy of the benchmark. \$\endgroup\$
    – Cody Gray
    Commented Apr 10, 2022 at 9:40
54
\$\begingroup\$

I’m not going to bother with a full review since the OP is already moving on to v2 of this project. But I think there are some very interesting things that could be helpful to many people still worth noting.

I am going to focus very specifically on just the numeric conversions, both integral and floating point. I am only going to consider the constructors and set() functions—and prepend() and append()—for API considerations; I’m not going to bother digging into the guts of the xstring class.

Before we get started on that, though, there’s one item of housekeeping to cover.

Don’t use macros

Seriously, don’t. If you never write another #define, you will be a better C++ coder.

I can’t believe there’s anyone out there still teaching this kind of use of the preprocessor in 2022. If you learned this stuff from a book, throw it out, and get a new one.

Every single use of the preprocessor in this code—except for #includes (and even that is something that will soon be obsolete)—is wrong.

For example, this:

// xstring macros
#define xnpos            SIZE_MAX  // max size of size_t, when error, return to it
#define _xs_extra_space_ 25        // the extra space size in heap memory

should be this:

// xstring ***CONSTANTS***
inline constexpr auto xnpos = SIZE_MAX;                     // max size of size_t, when error, return to it
inline constexpr auto _xs_extra_space_ = std::size_t(25);   // the extra space size in heap memory

And even then, they’re both still wrong. Both should be within the xstring class, not at global scope. And on top of that, _xs_extra_space_ is an illegal name at top level scope, because of the leading underscore.

But this will be a recurring theme: macros are bad; stop using them; never use them again.

So let’s get into it.

Integer conversions

All of the integer conversion stuff is in the x::hlpr namespace. (But why hlpr? Why not helper? Code won’t run any faster if you remove the vowels.) Let’s start with just the length functions:

// xstring helpers
namespace x::hlpr
{
    // [INTEGER LENGTH]
    #define __sig_int_len__(t) (t int src)                   { unsigned t int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    #define __uns_int_len__(t) (unsigned t int src)          { unsigned t int len = 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    unsigned int intlen __sig_int_len__() 
    unsigned int uintlen __uns_int_len__() 
    unsigned short int sintlen __sig_int_len__(short) 
    unsigned short int usintlen __uns_int_len__(short) 
    unsigned long lintlen __sig_int_len__(long) 
    unsigned long ulintlen __uns_int_len__(long) 
    unsigned long long llintlen __sig_int_len__(long long) 
    unsigned long long ullintlen __uns_int_len__(long long)

    // ... [snip] ...
}

Okay, problem #1: you are using illegal identifiers. You cannot use identifiers with double underscores, period. Those are reserved for the implementation. You may get away with it for a while, but you’re breaking the rules, and playing with fire.

Problem #2: you’re using macros. This pattern you’re using has not been acceptable in C++ since the 1980s.

Problem #3: even more bizarrely, you’re using archaic, C-style type prefixes, rather than function overloading. This is C++, not C. We don’t write intlen(i), uintlen(u), sintlent(s), and so on. We write intlen(i), intlen(u), intlen(s), and so on.

So before we even get to the real problems, we have to clean this mess up quite a bit:

namespace x::hlpr
{
    // [INTEGER LENGTH]
    #define SIG_INT_LEN(t) (t int src)                   { unsigned t int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    #define UNS_INT_LEN(t) (unsigned t int src)          { unsigned t int len = 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    unsigned int        intlen SIG_INT_LEN() 
    unsigned int        intlen UNS_INT_LEN() 
    unsigned short int  intlen SIG_INT_LEN(short) 
    unsigned short int  intlen UNS_INT_LEN(short) 
    unsigned long       intlen SIG_INT_LEN(long) 
    unsigned long       intlen UNS_INT_LEN(long) 
    unsigned long long  intlen SIG_INT_LEN(long long) 
    unsigned long long  intlen UNS_INT_LEN(long long)

    // ... [snip] ...
}

Already this is a massive improvement, because now we don’t need to write 8 different functions for every mix of int, unsigned, short and long.

Which brings us to the first design question: Why return a different type for each of these functions? All they do is count the number of decimal digits in a number. There’s no reason that you need to return an unsigned long long when counting the digits in an unsigned long long. Even for a 64-bit number, you’re only going to have ~20 or so digits. Even for a 256-bit number, it’s going to be ~80, max. You could get away with returning signed char for every one of these functions (I guess until someone rolls out a int512_t). You might as well just return int:

namespace x::hlpr
{
    // [INTEGER LENGTH]
    #define SIG_INT_LEN(t) (t int src)                   { int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    #define UNS_INT_LEN(t) (unsigned t int src)          { int len = 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    int intlen SIG_INT_LEN() 
    int intlen UNS_INT_LEN() 
    int intlen SIG_INT_LEN(short) 
    int intlen UNS_INT_LEN(short) 
    int intlen SIG_INT_LEN(long) 
    int intlen UNS_INT_LEN(long) 
    int intlen SIG_INT_LEN(long long) 
    int intlen UNS_INT_LEN(long long)

    // ... [snip] ...
}

That is much simpler, and much easier to work into generic code.

But there are still several major problems here:

  1. You have restricted the set of types.

    These functions cover int, unsigned int, short, unsigned short, long, unsigned long, long long, and unsigned long long. So, you got ’em all, right?

    Wait, I don’t see signed char or unsigned char there. So, what, we can’t convert byte values to strings? Seems an odd restriction.

    I also don’t see std::size_t, or std::ptrdiff_t, or std::intmax_t.

    Nor do I see std::uint32_t, std::int_fast64_t, or any of their siblings.

    And I also don’t see support for extended integer types, like int128_t.

  2. You have bloated the code.

    There are 8 functions declared there, including for signed long long. But… what if you never use any long long values anywhere? Tough cookies; depending on how the program is built, you could end up stuck with a bunch of unused functions in your exe or library.

    That’s one of the most powerful features of templates: nothing gets instantiated unless it gets used (or you explicitly request its instantiation, of course).

Okay, but let me stop here and rewind. Let’s go back to your original code, with all its problems, and laser in our focus on just intlen():

namespace x::hlpr
{
    // [INTEGER LENGTH]
    #define __sig_int_len__(t) (t int src)                   { unsigned t int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }
    // ... [snip] ...
    unsigned int intlen __sig_int_len__()

    // ... [snip] ...
}

Here’s a multi-billion dollar question for you: does this code work?

Well? Does it?

If you’re sitting there, scratching your head, wondering if I’m about to spring some gotcha on you… well, you’ve already failed as a programmer. Because you should be able to answer that question confidently “yes”. The fact that you can’t means you haven’t done your job. Seriously, what kind of programmer hands you a program, and then when you ask them, “does it work?” just shrugs and says, “maybe!”

The reason you can’t answer confidently yes… is because you didn’t test your code. That should be step 1. No, scratch that, that should be step 0. Good practice is to write your tests before you write your code.

I’ll illustrate using Boost.Test, but it doesn’t really matter what test framework you use. I just know Boost.Test inside and out, and it’s by far the most powerful framework out there (though, not the easiest to get started with!).

Most of the following code is just Boost.Test boilerplate, so I’ll gloss over it and highlight the important stuff.

So, first I create a header intlen.hpp, and put just the lines above there:

#ifndef INDI_INC_intlen
#define INDI_INC_intlen

namespace x::hlpr
{
    #define __sig_int_len__(t) (t int src)                   { unsigned t int len = src < 0 ? 1 : 0;  while(true) { src /= 10; if(src != 0) len ++; else return len; } }

    unsigned int intlen __sig_int_len__()

}

#endif // include guard

Then I create a test file intlen.test.cpp:

// Have to define the test module first, always.
#define BOOST_TEST_MODULE intlen

// You just need to include boost/test/included/unit_test.hpp...
//
// ... HOWEVER...
//
// If you do the following trick, it can speed up your tests *MASSIVELY*.
#ifdef BOOST_TEST_DYN_LINK
#   include <boost/test/unit_test.hpp>
#else
#   include <boost/test/included/unit_test.hpp>
#endif // BOOST_TEST_DYN_LINK

// Need these for the actual test stuff.
#include <array>
#include <limits>
#include <string>

// Need these so we can use data-driven tests.
#include <boost/test/data/test_case.hpp>
#include <boost/test/data/monomorphic.hpp>

// Obviously we need to include the header with the function we're testing.
#include "intlen.hpp"

// Now finally we get to the interesting stuff.
//
// Here we define a bunch of int values we are going to test. We don’t need
// to test *EVERY* possible int value, but we should check all the potential
// edge cases.
constexpr auto test_data = std::array{
    // Zero is likely to be interesting/unique.
    0,
    // One and negative one might also be interesting.
    1,
    -1,
    // The max and min values of int are obviously edge cases.
    std::numeric_limits<int>::max(),
    std::numeric_limits<int>::min(),

    // The above is all we really need, but we can add random values here
    // if we want. (We could also use Boost.Test to generate random values
    // but that's too much for this case.)
    123,
    -123,
    12345,
    -12345
};

// Now that we have a bunch of test case values above, we use this to generate
// the actual test cases:
BOOST_DATA_TEST_CASE(
    // name of the test case:
    intlen_result,
    // test case values:
    boost::unit_test::data::make(test_data),
    // name of the value in each test
    value)
{
    // And the actual test is simple. We use to_string() to convert the
    // value to a string, and then get its length. That is the expected
    // value that intlen() should return.

    auto const expected = std::to_string(value).size();

    BOOST_TEST(intlen(value) == expected);
}

Now just run the tests and…

Well, well, well.

Running 9 test cases...
prog.cc(67): error: in "intlen_result/_0": check x::hlpr::intlen(value) == expected has failed [0 != 1]
Failure occurred in a following context:
    value = 0; 
prog.cc(67): error: in "intlen_result/_1": check x::hlpr::intlen(value) == expected has failed [0 != 1]
Failure occurred in a following context:
    value = 1; 
prog.cc(67): error: in "intlen_result/_2": check x::hlpr::intlen(value) == expected has failed [1 != 2]
Failure occurred in a following context:
    value = -1; 
prog.cc(67): error: in "intlen_result/_3": check x::hlpr::intlen(value) == expected has failed [9 != 10]
Failure occurred in a following context:
    value = 2147483647; 
prog.cc(67): error: in "intlen_result/_4": check x::hlpr::intlen(value) == expected has failed [10 != 11]
Failure occurred in a following context:
    value = -2147483648; 
prog.cc(67): error: in "intlen_result/_5": check x::hlpr::intlen(value) == expected has failed [2 != 3]
Failure occurred in a following context:
    value = 123; 
prog.cc(67): error: in "intlen_result/_6": check x::hlpr::intlen(value) == expected has failed [3 != 4]
Failure occurred in a following context:
    value = -123; 
prog.cc(67): error: in "intlen_result/_7": check x::hlpr::intlen(value) == expected has failed [4 != 5]
Failure occurred in a following context:
    value = 12345; 
prog.cc(67): error: in "intlen_result/_8": check x::hlpr::intlen(value) == expected has failed [5 != 6]
Failure occurred in a following context:
    value = -12345; 

*** 9 failures are detected in the test module "intlen"

What you’re looking at there is 9 test cases… 9 failures.

In other words, your intlen() function is completely broken.

And you didn’t have a clue. Because you didn’t test.

Looking at the results, there is a pattern. The result of intlen(value) is always 1 less than the expected value.

Let’s write that macro expanded in the function, spread it out legibly, and investigate it:

unsigned int intlen(int src)
{
    unsigned int len = src < 0 ? 1 : 0;

    while (true)
    {
        src /= 10;

        if(src != 0)
            len ++;
        else
            return len;
    }
}

Now the problem is plain as day to see. If src is zero then len is zero before the loop. On the first run through the loop, src /= 10; just leaves src as zero, so the if is not triggered, and… we return zero. So intlen(0) returns 0. But… 0 is one digit long.

It’s a stupidly simple bug, and a stupidly simple fix: you just need to do the if test after incrementing len.

So I edit intlen.hppnote: I don’t touch the tests!—and replace the macro stuff with this:

namespace x::hlpr {

inline unsigned int intlen(int src)
{
    unsigned int len = src < 0 ? 1 : 0;

    while (true)
    {
        src /= 10;
        ++len;

        if (src == 0)
            return len;
    }
}

} // namespace x::hlpr

Now I run the tests, and…:

Running 9 test cases...

*** No errors detected

Easy peasy.

See how easy it was to detect, diagnose, and fix a bug, with tests? Not only that, I can now say with confidence: this function works.

And if at some future date, someone identifies another bug, like “seems to fail with the number 32”, I can just make a new test case to capture the bug, then fix it so that all tests pass again, and be even more confident that the function is correct. Without tests, any time you touch that function, you can’t be sure you haven’t broken it, so you can never be confident about it.

All that and we haven’t even needed to touch a debugger.

TEST. YOUR. CODE. That is not just a suggestion. If you are not testing your code, your code is garbage; it is absolutely useless in any serious project. No one wants to put code in their project where the author says, “well, shrug, I think it works”.

And now that we have tests, we can freely refactor the code. So let’s do that. We’ve already fixed the illegal identifier, and removed the macro. So let’s go all-in, and make it a template:

namespace x::hlpr {

template <typename T>
int intlen(T value)
{
    int length = int(value < 0);

    do
    {
        value /= 10;
        ++length;
    } while (value != 0);

    return length;
}

} // namespace x::hlpr

Run the tests again to make sure everything still works, and… whoops, warnings about comparing signed and unsigned values, because I switched from returning unsigned int to just int. That’s fair, but returning int is the right thing to do in any case, for multiple reasons (for example).

No problem. In the test, just change the line auto const expected = std::to_string(value).size(); to auto const expected = int(std::to_string(value).size()); (just add an int conversion), run the tests again, and, we’re good.

And not just good. This single intlen() function now works… correctly… for all signed integer types. ALL of them, including extended types. And you know it works correctly, because it’s tested. (Although, there’s no harm in testing it with other integer types if you’re concerned. Probably a good idea to try it with signed char and/or short at least, to make sure it behaves properly even when arithmetic promotions are happening all over the place.)

At this point, we’ve brought this code into the 1990s. Now it’s time to bring it into the 21st century.

First, let’s make it constexpr.

Second, let’s mark it noexcept, because there is no way it can fail.

Third, let’s constrain it.

The simplest way to constrain it is using the std::signed_integral concept. The problem with that is that std::signed_integral includes not just all the signed integrals… it includes all the character types (char, char8_t, wchar_t, assuming they’re signed), and bool. We don’t want that. So we need to make new concepts that are more what we want.

So, first we make a concept that tests for character types:

template <typename T>
concept character =
    std::same_as<T, char>
    or std::same_as<T, char8_t>
    or std::same_as<T, char16_t>
    or std::same_as<T, char32_t>
    or std::same_as<T, wchar_t>;

That’s handy to have in any case.

With that, we can make our new integer concepts:

template <typename T>
concept signed_integer =
    std::signed_integral<T>
    and not character<T>
    and not std::same_as<T, bool>;

template <typename T>
concept unsigned_integer =
    std::unsigned_integral<T>
    and not character<T>
    and not std::same_as<T, bool>;

template <typename T>
concept integer = signed_integer<T> or unsigned_integer<T>;

And now we can constrain our function properly:

#include <concepts>

namespace x::hlpr {

template <typename T>
concept character =
    std::same_as<T, char>
    or std::same_as<T, char8_t>
    or std::same_as<T, char16_t>
    or std::same_as<T, char32_t>
    or std::same_as<T, wchar_t>;

template <typename T>
concept signed_integer =
    std::signed_integral<T>
    and not character<T>
    and not std::same_as<T, bool>;

template <typename T>
concept unsigned_integer =
    std::unsigned_integral<T>
    and not character<T>
    and not std::same_as<T, bool>;

template <typename T>
concept integer = signed_integer<T> or unsigned_integer<T>;

constexpr auto intlen(signed_integer auto value) noexcept -> int
{
    auto length = int(value < 0);

    do
    {
        value /= 10;
        ++length;
    } while (value != 0);

    return length;
}

} // namespace x::hlpr

Run tests. Success.

So we have eliminated a macro and 4 redundant functions, fixed a bug, and ended up with a constrained template function that will work for all signed integer types. (We also created 4 handy concepts along the way, which will be very useful later.)

I’m not going to repreat the process for the unsigned integer macro/functions, because it’s literally identical. In fact the only differences between the final unsigned function and the one above are:

  1. It’s constrained with unsigned_integer (rather than signed_integer).
  2. The first line is just auto length = 0;, because value < 0 will never be true for an unsigned type.

In fact, you might be able to roll both into a single function. I say “might” because a compiler might complain about the value < 0 test for unsigned types, because it is always false. You won’t know till you try it! Run the tests on the all the compilers you want to support! See what happens!

So let’s call intlen() a solved problem for now, and move on. But rather than just blindly move on to the next macro/function, let’s think about the end goal.

Your plan is basically this:

  1. Get the string length of a number.
  2. Allocate that amount of memory.
  3. Parse the string into it.

It’s not a bad plan, but there are a couple of things to consider.

The first is that while the memory allocation is far and away the largest cost, parsing is the next largest cost, due to all the divisions/modulos (divisions aint’t cheap). You’re basically parsing the number twice: once to get the length, then once again to actually parse it.

The other thing to consider is that even in the worst case, the string length isn’t going to be large. For a 32 bit integer, the max size is just 11 (10 digits + the sign). That’s nothin’ man. And, realistically speaking, when you allocate memory, many (most?) allocators aren’t going to allocate just 2 or 3 bytes; you’re probably going to get a minimum allocation of 8 to be worthwhile. (And if you’re using the small-string optimization, anything less than ~15 is free anyway.)

So here’s an alternate strategy:

  1. Allocate the worst-case for a number.
  2. Parse the string into that.

We save a step! We maybe overallocate, but that memory can still be used (it’s extra capacity), and if there’s a small-string optimization going on, it’s probably a wash anyway.

If you’re really concerned about memory—say, in cases where the worst-case is larger than the SSO size—you can always use a temporary buffer, parse into that, get the parsed size, allocate that, then copy. Even that might still be faster than double-parsing to precompute the size. And you can always use a heuristic to decide when to do that.

Let’s try implementing the simple case for now. Let’s try implementing xstring::set() for integers:

// Current implementation:
//  bool xstring::set(int ref) { __intstr__(ref) return set(__ref, __len); }
// I have literally no idea what is supposed to be happening there, because
// it's all obscured by macros. (And virtually every identifier there is
// illegal.)

// Not sure why returns bool; don't care.
auto xstring::set(integer auto i) -> bool
{
    // I'm ignoring any previous allocated memory.

    // First, determine the worst-case size.
    capacity = std::numeric_limits<decltype(i)>::digits10
        + 1                             // for last digit
        + signed_integer<decltype(i)>   // sign, if any
    ;

    // Allocate that.
    auto p = new char[capacity + 1];

    // Parse the number into that memory.
    //
    // Use your own parse function, or:
    if (auto const [ep, ec] std::to_chars(p, p + capacity, i); ec == std::errc{})
        length = ep - p;
    else
        throw std::system_error{make_error_code(ec)};

    // And you’re done.
    return true;
}

// Note about numeric_limits<T>::digits10:
//
// numeric_limits<T>::digits10 can be confusing, because it works backwards.
// It doesn't tell you how many base-10 digits you need for T, it tells you
// how many base-10 digits T fully supports.
//
// For example, uint8_t supports values from 0 to 255. That means
// numeric_limits<uint8_t>::digits10 is 2. Because uint8_t can fully represent
// all 2-digit base-10 numbers, from 00 to 99. But it *can't* represent all
// 3-digit base-10 numbers: it can't represent 431 for example.
//
// So you need to add 1 to numeric_limits<T>::digits10 to account for the last
// digit.

Or, even simpler:

auto xstring::set(integer auto i) -> bool
{
    // Prepare a worst-case sized buffer.
    constexpr buffer_size = std::numeric_limits<decltype(i)>::digits10
        + 1                             // for last digit
        + signed_integer<decltype(i)>;  // for sign, if any

    // std::abs() won't be constexpr until C++23
    auto const abs = [](integer auto i) { return (i < 0) ? -i : i; };

    auto buffer = std::array<char, buffer_size>{};

    // Parse the number into it *backwards*.
    auto const negative = i < 0;
    
    auto const e = buffer.data() + buffer.size();
    auto b = e;

    do
    {
        *(--b) = char(abs(i % 10) + '0');
        i /= 10;
    } while (i != 0);

    if (negative)
        *(--b) = '-';

    // Create a string view of what was parsed.
    auto const sv = std::string_view(b, e);

    // Now just pass that on to another function, to handle all the resizing
    // logic and whatnot.
    return set(sv);
}

The first chunk of that function could be refactored out and reused, like so:

// This function can be tested up the wazoo, because it is isolated and
// independent. Once you’re sure it works, you will know *all* the integer
// parsing functions in the string class work. (At least, you’ll know the
// parsing part works; you still need to test that the memory stuff is okay.
// But at least that's all localized in a single overload of
// set()/prepend()/append()/whatever now.)
constexpr auto parse_integer(integer auto i) noexcept
{
    // Get the integer type.
    using int_t = decltype(i);

    // std::abs() won't be constexpr until C++23
    auto const abs = [](integer auto i) { return (i < 0) ? -i : i; };

    // Calculate the buffer size needed.
    constexpr auto buffer_size = std::numeric_limits<int_t>::digits10 + 1 + signed_integer<int_t>;

    // Set up the buffer type.
    using buffer_t = std::array<char, buffer_size>;

    // Create a NVRO result value with the buffer and the start position.
    auto result = std::tuple<buffer_t, int>{};

    // Get some handy shorthand variables.
    auto& buffer = std::get<0>(result);
    auto& start = std::get<1>(result);

    // Now just parse as before right into the return value buffer.
    auto const negative = i < 0;
    
    auto const e = buffer.data() + buffer.size();
    auto b = e;

    do
    {
        *(--b) = char(abs(i % 10) + '0');
        i /= 10;
    } while (i != 0);

    if (negative)
        *(--b) = '-';

    // Save the start position in the buffer.
    start = e - b;

    return result;
}

auto xstring::set(integer auto i) -> bool
{
    auto const [buffer, start] = parse_integer(i);
    auto const sv = std::string_view(buffer.data() + start, buffer.data() + buffer.size());

    return set(sv);
}

auto xstring::append(integer auto i) -> bool
{
    auto const [buffer, start] = parse_integer(i);
    auto const sv = std::string_view(buffer.data() + start, buffer.data() + buffer.size());

    return append(sv);
}

// ... and so on...

That’s about all there is to be said for the integer conversions. There’s plenty more improvements that could be made, but at this point, it’s just details.

I will say, though, that if you’re going to use your existing parsing functions (__sig_int_str__ and __sig_int_str__), you really should test them. There are more bugs lurking in there that I haven’t mentioned.

But of course, you should be testing everything anyway.

Floating point conversions

Now, I’m not going to pick apart the floating point conversions here. I’ve already demonstrated that they’re broken. And in any case… trying to pick apart a 1500 character macro… no… just no… I’d rather drill a hole in my head with a spoon.

Instead, I’ll tell you a true life story about floating point conversions.

Converting numbers to strings has always been a slog in C++, because you had to use C++ streams… which are expensive and syntax heavy… or C library functions… which are just about unusable in non-trivial, modern programs. The root problem, in both cases, is that all conversions used locales… which is fine for some numeric conversions, particularly those you want to show to the user… but a lot of the time is just pointless and unnecessary, because you’re converting numbers for some data file, and they don’t need to be prettily formatted. About 90% of the numeric conversion code I wrote before C++17 looked like this:

// I have some number...
auto const number = 12345;

// And I just want to put it in this string: "code = 12345"
//
// And I don't want it to format as: "code = 12,345"
// or in Germany as                : "code = 12.345"
// or in Japan as                  : "code = 一万二千三百四十五"

// You may be thinking, no problem, just use to_string().
//
// Nope! Trap! std::to_string() uses the current locale. So unless you change
// the current *GLOBAL* locale, you have no idea what you’ll get from
// std::to_string().
//
// (And you should *never* fuck with the global locale, except at the highest
// program-level scope. That way lies madness.)

// So first I have to create a custom-purpose stream:
auto oss = std::ostringstream{};

// And I have to remember to imbue it with the classic/POSIX/C locale:
oss.imbue(std::locale::classic());

// *NOW* I can finally format my number:
oss << number;

// And put together my string:
result = "code = " + oss.str();

Not only is that a lot of typing for a simple thing, it’s extremely inefficient. I need to create a whole string stream, and imbue it with a locale, just to format a single flippin’ number!

So C++17 brought along elementary string conversions, in the form of the <charconv> header, and the functions to_chars() and from_chars(). And, oh, these functions are wonderful. Their interface is a little clunky, but that’s because they’re hella efficient. They don’t allocate nothin’. They don’t use any locales or other esoteric crap. They take my number 12345, and they turn it into the characters "12345", no fuss, no muss, lighting fast, with no wasted cycles or memory.

So… what’s the problem?

Well, take a look at this table showing support for C++17 library features. Notice that almost all of C++17 is supported by the major compilers, and the few exceptions are extremely complicated features like the parallelized algorithms library, and the special math functions… things that require domain-specific PhD-level expertise to implement.

Notice, though, a few lines from the bottom, an interesting outlier. The line for the elementary string conversions has a lot of yellow: neither Clang nor C++Builder fully support it. Specifically, both Clang and C++Builder say “no FP from_chars”… but all compilers were “no FP” until recently. The “no FP” means “no floating point”, meaning all these libraries had integer conversions… but not floating point conversions.

Let that sink in. It is now 2022. Clang still doesn’t support converting floating point numbers to strings (well, as of like two weeks ago or so, it finally supports to strings… still not from). It took GCC 4 versions to finally get it right (11 is the latest version). Microsoft got it pretty quickly, but even they took a few releases. Borland/CodeGear/Embarcadero/whatever-the-hell-they’re-calling-themselves-these-days, they’re still hacking away at it.

Again, stop and let that sink in: what you’re trying to do… converting floating point numbers to strings… without any locale support… took the best C++ programmers in the world almost 5 years to get right… and some of them still haven’t got it done! That’s how mind-fuckingly hard this problem is.

Now, the best C++ programmers in the world took half a decade trying to convert floating point numbers to strings—and some are still working on it. So… riddle me this: do you think you managed to do it correctly? In, what, how many days did you spend on this code?

I’m just trying to impress upon you how spectacularly hard this problem is, and how absolutely impossible it is that you managed to solve it, when teams of the best C++ coders in the world haven’t been able to lick it for almost a half-decade.

So I don’t even need to look at your code; it’s wrong. I mean, I already checked, but, that was just a formality. This problem is just too hard to blindly hack into a solution.

If, after reading all that, you still want to take a swing at it… well, dude, you got stones. (I mean, assuming you’re a dude. If not, you got the the gender-relative variant of stones. Which are probably still basically “stones”, just in a different place with a different technical biological name.)

So if you’re really gonna do it, here’s what I recommend.

First, break this out of the string stuff, and make it its own unit. Even if you only get this partially working, it’s going to be a wicked library to have on its own, even outside of the string class.

Second, write an arsenal of tests for what you expect to get, and write them first. The reason is that there is no one “correct” way to format floats. 1.0 could be equally well formatted as "1" (which streams will do by default), "1.00000" (the default for to_string()), "1.0" (the “sensible” representation for 1.0), or "1e0" (unusual, but still correct). It’s your library, so you get to decide what’s right.

Don’t forget the special case values, like nan and inf (both positive and negative)!

Basically, you want to write one big mother of an array of pairs, where each pair is a value and the string you expect it to format to, and then a data-driven test to check ’em all. In Boost.Test, that might look something like:

constexpr auto double_test_data = std::to_array<std::tuple<double, std::string_view>>({
    { 0.0, "0"sv },
    { 1.0, "1"sv },
    { 0.1, "0.1"sv },

    // ...

    { std::numeric_limits<double>::infinity(), "inf"sv },
    { -std::numeric_limits<double>::infinity(), "-inf"sv },

    // ... and so on
});

BOOST_DATA_TEST_CASE(
    double_to_string,
    boost::unit_test::data::make(double_test_data),
    value,
    expected)
{
    // Buffer to hold the conversion. 100 is plenty of space.
    auto buffer = std::array<char, 100>{};

    // Do the conversion (I'm just guessing at the interface).
    auto const end = x::float_to_string(buffer.data(), buffer.size(), value);

    // Get a string view of the converted characters.
    auto const result = std::string_view(buffer.data(), end);

    // Check it!
    BOOST_TEST(result == expected);
}

Other test frameworks will have similar constructs. Catch2, which is an excellent option, uses the concept of “generators”. I’m not really a Catch2 user, but I think it would look something like:

TEST_CASE("double to string")
{
    auto data = GENERATE(table<double, std::string_view>({
        // same data as above
    }));

    auto buffer = std::array<char, 100>{};

    auto const end = x::float_to_string(buffer.data(), buffer.size(), std::get<double>(data));

    auto const result = std::string_view(buffer.data(), end);

    REQUIRE(result == std::get<std::string_view>(data));
}

This won’t be an easy task, but if you can get it done, even partially… damn, that will be impressive. If you get it done really well, it might even be worth an article in whatever the current analogue of C++ User’s Journal is (maybe ACCU?), and possibly even a conference talk.

However… if this is not a path you want to start down, then I suggest throwing out your attempt, and instead just using the standard library for floating point conversions. std::to_chars() for floating point types is finally fairly-well supported, so you could use that. Or you could just use a stream.

Either way, good luck!

\$\endgroup\$
14
  • 4
    \$\begingroup\$ A quite impressive and very informational write-up. Well done! \$\endgroup\$ Commented Apr 10, 2022 at 17:36
  • 2
    \$\begingroup\$ The unsigned int len = src < 0 ? 1 : 0; in integer length stuff can go away if you use a do{}while(src != 0) loop. 0 / 10 is still zero, but does one loop iteration. See How do I print an integer in Assembly Level Programming without printf from the c library? for C and x86 asm implementations. (Converting into a small tmp buffer and copying once you know the length would likely be more efficient that running through the chain of division-by-multiplicative-inverse twice, despite the store-forwarding stall in memcpy reloading byte stores) \$\endgroup\$ Commented Apr 11, 2022 at 0:03
  • 3
    \$\begingroup\$ Your section about FP<->string conversions seems a bit hyperbolic. Yes, it's hard, but the algorithms have already been developed. (And written about, e.g. exploringbinary.com/topics has a whole series of articles about it, e.g. exploringbinary.com/how-strtod-works-and-sometimes-doesnt about David Gay's strtod, and another about glibc's. And articles about FP->string, such as exploringbinary.com/… - highly non-trivial too to correctly round-trip) \$\endgroup\$ Commented Apr 11, 2022 at 0:16
  • 5
    \$\begingroup\$ @PeterCordes I think it's safe to say FP string conversions are stupidly hard. IIRC Stephan T. Lavavej gave a talk on the topic: every library used some form of bignum, and it's only recently an algorithm was discovered that doesn't. \$\endgroup\$
    – Passer By
    Commented Apr 11, 2022 at 2:05
  • 5
    \$\begingroup\$ @PasserBy: Absolutely agreed with that, but claiming "it took the best C++ programmers in the world almost 5 years to get right… and some of them still haven’t got it done!" implies that people were actively working on this continuously for 5 years. When in reality, it's more likely that it's known to be hard so got a low priority for even starting. That's what I meant by a bit hyperbolic. Only a bit, not a lot, the overall sentiment is fair. Thanks for the note about it being possible without bignum, that's cool, will have to check that out some time. \$\endgroup\$ Commented Apr 11, 2022 at 2:13
28
\$\begingroup\$

General Advice

Writing a new string class is a very common exercise. You are probably not going to be able to beat the performance of the STL unless you change the contract of the STL—either the API itself, or the guarantees it gives—in a way that enables optimizations that the STL is not allowed to use. People have been working hard for twenty years at making std::string as fast as it can be, given the trade-offs the design committee chose.

One example of this is that it is not trivial at all to implement case-conversion for strings stored as UTF-8, given that the uppercase and lowercase equivalent of a UTF-8 grapheme might have different sizes, nor to search the base characters in a string ignoring accents (such as the pattern fiance matching fiance, fiancé or fiancée). If you are implementing something like that, and can drop compatibility with all the other encodings that std::string must support, it could be worthwhile to write your own class that always stores and maintains its data in NFD-normalized UTF-8.

Another example is if you want to implement a Rope class. It is not possible to implement std::string as a rope, because the Standard guarantees that a std::string always stores its data in contiguous memory. Note that, if you violate this assumption, you will no longer be able to efficiently convert your Rope to either a C-style string or a view of the string. But this data structure could be optimal for many problems, such as lazily-evaluated I/O.

If you don’t have a specific application in mind that makes different trade-offs than std::string, it might still be worthwhile reimplementing it yourself, to get a good idea of how to write a high-performance container and why the design is the way it is.

Specific Suggestions

Several features that would be important to real-world code are missing here, including:

  • This implementation assumes an obsolete 8-bit character encoding and does not support any form of Unicode
  • It does not enable views or substrings of an xstring
  • It does not accept any standard string type as a parameter, other than a char *
  • This is especially inefficient because it always calls strlen to count the number of characters in its C-style input argument, in linear time, and this is the only common format it recognizes
  • It is not moveable
  • It does not have short-string optimization
  • Virtually all operations are destructive
  • There are no optimizations for the case where an argument is an xstring&&, which would allow you (for example) to write your current prepend and append as special cases of xstring1 + xstring2 that resize whichever of the two strings can be overwritten, greatly simplifying your API
  • There is no way to reserve memory for an xstring that you expect to fill in later, or shrink it to fit when you are done modifying it
  • There is no list-concatenate operation that precalculates the final size of the string, potentially avoiding multiple resizes and copies
  • No operations are inline or constexpr to enable optimization
  • There is no overload either to change the allocator (e.g. to allocate a string from shared memory) or to provide a custom allocator object (e.g. to allocate strings from thread-local pools of memory and have them remember which pool they belong to).

So, for example, this API gives you no way to extract a word from a user-supplied string and combine it with an xstring, other than to copy the substring to a null-terminated C-style string or xstring. If you want to add it multiple times, it must recalculate the size of the argument each time. This would be vastly more efficient if your API accepted std::string_view or a substring type. It misses many opportunities to automatically elide copies. This API is fundamentally going to be much slower for any application that uses many short strings.

I haven’t gone through and checked the implementation for bugs (although C-style manual memory management is very tricky to get right), but I would recommend a major redesign of the API.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for these suggestions I've received a lot of similar messages since posting the code, so I'm making some edits now, thank you \$\endgroup\$ Commented Apr 10, 2022 at 0:34
  • \$\begingroup\$ @xeerx Glad they were helpful. However, it’s generally considered poor form to edit a question in a way that invalidates existing answers. \$\endgroup\$
    – Davislor
    Commented Apr 10, 2022 at 5:41
  • \$\begingroup\$ @xeerx I’ve edited in some more general advice for you and the many other great minds who have thought alike. (As well as myself in hindsight.) \$\endgroup\$
    – Davislor
    Commented Apr 10, 2022 at 6:11
11
\$\begingroup\$

Correctness

The correctness of code is always important when it compares with the speed / performance. There is nothing useful for running fast and generating unexpected results, especially dealing with the real world problems. It seems that there are some issues in the part of integer and floating point conversion in your implementation. The content in test.cpp mainly performs the speed testing. In order to verify the correctness, I am trying to follow the way you using xstring object and the test code snippet is as follows:

//  test with string
std::cout << "***test with string***\n";
std::string s_string = "test for std::string";
std::cout << s_string << '\n';

xstring x_string = "test for xstring";
std::cout << x_string << '\n';
std::cout << "\n\n";

//  test with character
std::cout << "***test with character***\n";
std::string s_character = std::to_string('c');
std::cout << "std::string output: " << s_character << '\n';
xstring x_character = 'c';
std::cout << "xstring output: " << x_character << '\n';
std::cout << "\n\n";

//  test with bool
std::cout << "***test with bool***\n";
std::string s_bool = std::to_string(true);
std::cout << "std::string output: " << s_bool << '\n';
xstring x_bool = true;
std::cout << "xstring output: " << x_bool << '\n';
std::cout << "\n\n";

//  test with integer
std::cout << "***test with integer***\n";
std::string s_integer = std::to_string(1);
std::cout << "std::string output: " << s_integer << '\n';

xstring x_integer = 1;
std::cout << "xstring output: " << x_integer << '\n';
std::cout << "\n\n";

// test with decimal
std::cout << "***test with decimal***\n";
std::string s_decimal = std::to_string(123.456);
std::cout << "std::string output: " << s_decimal << '\n';

xstring x_decimal = 123.456;
std::cout << "xstring output:" << x_decimal << '\n';

Then, the results of integer and decimal cases can't be printed.

***test with string***
test for std::string
test for xstring


***test with character***
std::string output: 99
xstring output: c


***test with bool***
std::string output: 1
xstring output: 1


***test with integer***
std::string output: 1
xstring output: 


***test with decimal***
std::string output: 123.456000
xstring output:

The godbolt link is available here.

\$\endgroup\$
5
  • \$\begingroup\$ The code is being improved, please wait one more day and try it again, thank you \$\endgroup\$ Commented Apr 10, 2022 at 1:24
  • 6
    \$\begingroup\$ @xeerx Thank you for your replying. Please notice that Do not change the code in the question after receiving an answer.. Thank you. \$\endgroup\$
    – JimmyHu
    Commented Apr 10, 2022 at 1:33
  • 1
    \$\begingroup\$ No, I'm just going to add some improvements and I'll tell you about them in the post \$\endgroup\$ Commented Apr 10, 2022 at 1:34
  • 3
    \$\begingroup\$ Neither std::to_string('c') nor std::to_string(true) do what you think they do. There are no to_string() overloads for char or bool. In both cases, those literals are being converted to int (which you can see when the 'c' becomes 99). To convert a char to a string, you do string(1, 'c'). To convert bool, you’d have to do something like true ? "true"s : "false"s, or use a stream or use_facet/numpunct yadda yadda. \$\endgroup\$
    – indi
    Commented Apr 10, 2022 at 1:54
  • \$\begingroup\$ @indi Indeed, it seems that it's rare to use bool or char in std::string type. I just make the structure of the test code above follows the OP's usage in test.cpp. \$\endgroup\$
    – JimmyHu
    Commented Apr 10, 2022 at 3:37
10
\$\begingroup\$

The reason lies in the way I handle the heap memory, I don't need to realloc memory every time the value changes, and I don't need to completely reset the object when any modification is made.

std::string, used correctly, does none of those things either... So, my review would end right here: just use std::string.


It takes a silly amount of work to implement even a partial replacement for std::string that is correct. The time spent writing test cases and other test harnesses is almost always several times longer than implementing the thing.

Even if your implementation had some merits, without a test suite we mostly got your word for it that it works, and the reality with such code is that it will be buggy enough to be a source of constant, big frustration.

Such foundational code needs to be extremely robust. Extensive automated tests, including both classical test suites and fuzzing, and/or formal proofs of correctness are more important than the code itself, for without them, the code is worth not much.

\$\endgroup\$
2
  • \$\begingroup\$ Writing test harnesses? I had assumed the glibc tests are Open Source; your tests could be derived from them. \$\endgroup\$
    – MSalters
    Commented Apr 11, 2022 at 14:28
  • \$\begingroup\$ @MSalters Well yes, but if you want something not quite like std::string but "better" - whatever that may mean - you can be inspired by glibc and MSVC-STL test suites, but still have to roll your own. And those test suites are not for the faint of heart to begin with :) \$\endgroup\$ Commented Apr 12, 2022 at 21:44
4
\$\begingroup\$

One final thing, code formatting (I don't think anyone's mentioned it directly)

bool prepend    (float       ref,unsigned short max = 1) { __floatstr__   (ref) return prepend(__ref, __len);               }

Wouldn't

bool prepend(float ref, unsigned short max = 1)
{
// is this line correct ? 
  __floatstr__   (ref)

  return prepend(__ref, __len);
}

Look a bit nicer ?

\$\endgroup\$

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.