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 :
Are there any errors that I did not notice?
Is there anything that can be done to improve performance?
Is there anything missing?
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.
-
\$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$Mast– Mast ♦2022年04月12日 10:10:27 +00:00Commented Apr 12, 2022 at 10:10
6 Answers 6
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 | |
+-------------------------+--------+------+------------------------------+
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
.
-
1\$\begingroup\$ Interestingly enough,
icx
andclang
with-O3
and 2020 C++ standard are much faster for both versions, while still your concern stays true where the difference is measurable. \$\endgroup\$Oleg Lobachev– Oleg Lobachev2022年04月09日 13:01:59 +00:00Commented 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– indi2022年04月09日 16:41:52 +00:00Commented 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– indi2022年04月10日 01:34:05 +00:00Commented 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 stringsstd::string
stores them locally. It is achieved by either memory reporposing, or adding an extra buffer, or both. \$\endgroup\$ALX23z– ALX23z2022年04月10日 06:54:52 +00:00Commented 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– Cody Gray2022年04月10日 09:40:42 +00:00Commented Apr 10, 2022 at 9:40
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:
You have restricted the set of types.
These functions cover
int
,unsigned int
,short
,unsigned short
,long
,unsigned long
,long long
, andunsigned long long
. So, you got ’em all, right?Wait, I don’t see
signed char
orunsigned char
there. So, what, we can’t convert byte values to strings? Seems an odd restriction.I also don’t see
std::size_t
, orstd::ptrdiff_t
, orstd::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
.You have bloated the code.
There are 8 functions declared there, including for
signed long long
. But... what if you never use anylong 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...
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.hpp
—note: 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
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:
- It’s constrained with
unsigned_integer
(rather thansigned_integer
). - The first line is just
auto length = 0;
, becausevalue < 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:
- Get the string length of a number.
- Allocate that amount of memory.
- 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:
- Allocate the worst-case for a number.
- 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!
-
4\$\begingroup\$ A quite impressive and very informational write-up. Well done! \$\endgroup\$Captain Giraffe– Captain Giraffe2022年04月10日 17:36:23 +00:00Commented 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 ado{}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\$Peter Cordes– Peter Cordes2022年04月11日 00:03:01 +00:00Commented 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\$Peter Cordes– Peter Cordes2022年04月11日 00:16:03 +00:00Commented 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– Passer By2022年04月11日 02:05:11 +00:00Commented 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\$Peter Cordes– Peter Cordes2022年04月11日 02:13:15 +00:00Commented Apr 11, 2022 at 2:13
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 currentprepend
andappend
as special cases ofxstring1 + 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
orconstexpr
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.
-
\$\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\$Maysara Elshewehy– Maysara Elshewehy2022年04月10日 00:34:02 +00:00Commented 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– Davislor2022年04月10日 05:41:50 +00:00Commented 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– Davislor2022年04月10日 06:11:09 +00:00Commented Apr 10, 2022 at 6:11
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:
-
\$\begingroup\$ The code is being improved, please wait one more day and try it again, thank you \$\endgroup\$Maysara Elshewehy– Maysara Elshewehy2022年04月10日 01:24:42 +00:00Commented 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– JimmyHu2022年04月10日 01:33:06 +00:00Commented 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\$Maysara Elshewehy– Maysara Elshewehy2022年04月10日 01:34:49 +00:00Commented Apr 10, 2022 at 1:34
-
3\$\begingroup\$ Neither
std::to_string('c')
norstd::to_string(true)
do what you think they do. There are noto_string()
overloads forchar
orbool
. In both cases, those literals are being converted toint
(which you can see when the'c'
becomes99
). To convert achar
to a string, you dostring(1, 'c')
. To convertbool
, you’d have to do something liketrue ? "true"s : "false"s
, or use a stream oruse_facet
/numpunct
yadda yadda. \$\endgroup\$indi– indi2022年04月10日 01:54:11 +00:00Commented Apr 10, 2022 at 1:54 -
\$\begingroup\$ @indi Indeed, it seems that it's rare to use
bool
orchar
instd::string
type. I just make the structure of the test code above follows the OP's usage intest.cpp
. \$\endgroup\$JimmyHu– JimmyHu2022年04月10日 03:37:47 +00:00Commented Apr 10, 2022 at 3:37
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.
-
\$\begingroup\$ Writing test harnesses? I had assumed the glibc tests are Open Source; your tests could be derived from them. \$\endgroup\$MSalters– MSalters2022年04月11日 14:28:18 +00:00Commented 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\$Kuba hasn't forgotten Monica– Kuba hasn't forgotten Monica2022年04月12日 21:44:24 +00:00Commented Apr 12, 2022 at 21:44
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 ?