Написал свой string. Проверьте код на наличие подводных камней, утечек памяти и других ошибок

@vaziliybober · Регистрация: 02.01.2016

Author24 — интернет-сервис помощи студентам

Сделал класс, который ведет себя аналогично char*, но с возможностью сложения строк. Вроде все работает как надо, но есть ли какие-то подводные камни, утечки памяти? Может что-то сделано криво и можно поумнее? Дело в том, что на этот класс мне надо будет потом полностью полагаться и быть уверенным, что в нем нет ошибок. Вот код:

vstring.h

C++

class vstring
    {
    private:
        char* Str;
        int length;
    public:
        vstring(char* str = 0);
        vstring(char s);
        vstring(vstring& str);
 
        ~vstring(){delete [] Str;}
 
        char operator[](int index);
 
        friend std::ostream& operator<<(std::ostream& out, vstring& str);
 
        vstring operator+(vstring& str);
        vstring operator+(char symbol);
        friend vstring operator+(char symbol, vstring& str);
 
        operator char*();
 
        vstring& operator=(char* s);
        vstring& operator=(vstring& str);
         };

vstring.cpp

C++

vstring::vstring(char* str)
{
    if(str == 0)
    {
        Str = new char[1];
        length = 0;
        Str[0] = '\0';
        return;
    }
    for(length = 0; str[length] != '\0'; length++);
    Str = new char[length+1];
    Str[length] = '\0';
    for(int i(0); i < length; i++)
        Str[i] = str[i];
}
vstring::vstring(char s)
{
    Str = new char[2];
    length = 1;
    Str[1] = '\0';
    Str[0] = s;
}
 
vchar vstring::operator[](int index)
{
    return Str[index];
}
 
vstring::vstring(vstring& str)
{
    Str = new char [str.length+1];
 
    for(int i(0); i < str.length; i++)
        Str[i] = str[i];
    Str[str.length] = '\0';
    length = str.length;
}
 
std::ostream& operator<<(std::ostream& out, vstring& str)
{
    for(int i(0); i < str.length; i++)
        out << str[i];
    return out;
}
 
vstring operator+(char symbol, vstring& str)
{
    return vstring(symbol) + str;
}
 
vstring vstring::operator+(vstring& str)
{
    char* resStr = new char[length + str.length + 1];
 
    int i(0);
    for(; i < length; i++)
        resStr[i] = Str[i];
    for(; i < length + str.length; i++)
        resStr[i] = str[i-length]; 
    resStr[i] = '\0';
        
        vstring ret(resStr);
        delete [] resStr;
    return ret;
}
 
vstring vstring::operator+(char symbol)
{
    return *this + vstring(symbol);
}
 
vstring& vstring::operator=(char* s)
{
    delete [] Str;
 
    vstring str(s);
 
    Str = new char[str.length+1];
    for(int i(0); i < str.length; i++)
        Str[i] = str[i];
    Str[str.length] = '\0';
    length = str.length;
 
    return *this;
}
 
vstring& vstring::operator=(vstring& str)
{
    delete [] Str;
 
    Str = new char[str.length+1];
    for(int i(0); i < str.length; i++)
        Str[i] = str[i];
    Str[str.length] = '\0';
    length = str.length;
 
    return *this;
}
 
vstring::operator char*()
{
    char* str = new char[length+1];
    for(int i(0); i < length; i++)
        str[i] = Str[i];
    str[length] = '\0';
 
    return str;
}

@Dimension · 29.01.2016, 11:36

это все уже есть в string , зачем свои велосипед делать не понимаю

@vaziliybober · 29.01.2016, 11:43 **[ТС]**

Dimension, это для школы, мы там только велосипеды и изобретаем.

Croessmah · 29.01.2016, 11:47

C++

vstring(vstring& str);
vstring& operator=(vstring& str);
vstring(char* str = 0);
vstring& operator=(char* s);

Почему параметры не const?
Разве нельзя копировать константную строку?

C++

1	friend std::ostream& operator<<(std::ostream& out, vstring& str);

а константные строки выводу не подлежат?

C++

1	int length;

почему знаковый тип?
Думаете, длина строки когда-то будет меньше нуля?

C++

1	char operator[](int index);

опять же, почему параметр имеет знаковый тип?
Почему возвращается не ссылка на символ, а копия?

C++

1 2	vstring operator+(vstring& str); vstring operator+(char symbol);

Почему функции-члены не const?
Ну и ссылка в параметре тоже не const почему?

C++

1	operator char*();

хреновая идея. Возможны неявные касты,
а значит могут быть ошибки потом, которые черт поймаешь.
Лучше сделайте функцию-член c_str

C++

1	vchar vstring::operator[](int index)

что за vchar?
если это псевдоним,
то почему в объявлении char, а не vchar?

C++

1	for(length = 0; str[length] != '\0'; length++);

strlen и strcpy уже не в ходу?

C++

vstring::operator char*()
{
    char* str = new char[length+1];
    for(int i(0); i < length; i++)
        str[i] = Str[i];
    str[length] = '\0';
 
    return str;
}

Возвращаем копию строки, созданную динамически,
при этом заботу об освобождении возлагаем на клиента?
Фигня сразу. И спровоцирует чертову тучу утечек, т.к. неявный каст, см. выше.

C++

    Str = new char[str.length+1];
    for(int i(0); i < str.length; i++)
        Str[i] = str[i];
    Str[str.length] = '\0';
    length = str.length;

strcpy всё еще не в ходу.
Ну тогда хоть в свою функцию всё это дело оберните,
а то наплодили один и тот же код в тысячи местах. Не хорошо это

Вроде пока всё. Как исправите, приходите

@CheshireCat · 29.01.2016, 11:49

Ну, написание своих велосипедов - лучший способ обучения.

Касательно самого кода: он у меня даже не компиляется (GCC 4.9), поэтому в дальнейшем анализе на "какие-то подводные камни, утечки памяти" не вижу смысла.

@vaziliybober · 29.01.2016, 12:00 **[ТС]**

Croessmah, спасибо, буду исправлять, видите, все же надо иногда учиться на велосипедах.CheshireCat, думаю, это из-за того vchar вместо char (описка) в строке 24 vstring.cpp

@CheshireCat · 29.01.2016, 12:06

Да, одна из ошибок была именно про vchar. Исправляй, как только код скомпиляется, погляжу более подробно и попробую написать несколько тестов, "ломающих" твой код.

Croessmah · 29.01.2016, 12:10

Сообщение от vaziliybober

Croessmah, спасибо, буду исправлять, видите, все же надо иногда учиться на велосипедах.

Я этого не отрицал. Сам писал.

@vaziliybober · 29.01.2016, 21:30 **[ТС]**

Сообщение от CheshireCat

Исправляй, как только код скомпиляется, погляжу более подробно и попробую написать несколько тестов, "ломающих" твой код.

Если подключить <iostream> в vstring.h и "vstring.h" в vstring.cpp, то у меня на моем vs_2012 все компилится. Ну еще #pragma once можно на всякий. Мой косяк, не знаю, зачем я все это опустил.

Croessmah · 29.01.2016, 22:35

Сообщение от vaziliybober

Ну еще #pragma once можно на всякий

тогда можем попасть впросак,
если нет include guard'ов, т.к. прагмы не стандартны.
Также нет main'а, чтобы посмотреть как Вы всё это используете

@vaziliybober · 29.01.2016, 23:06 **[ТС]**

Сообщение от Croessmah

Как исправите, приходите

ну вот вроде все.

vstring.h

C++

#pragma once
#include <iostream>
 
class vstring
    {
    private:
        char* Str;
        unsigned int length;
 
        char* string_copy(const char* str);
        unsigned int string_length(const char* str);
    public:
        vstring(const char* str = 0);
        vstring(char s);
        vstring(const vstring& str);
 
        ~vstring(){delete [] Str;}
 
        char& operator[](unsigned int index) const;
 
        friend std::ostream& operator<<(std::ostream& out, const vstring& str);
 
        vstring operator+(const vstring& str) const;
        vstring operator+(char symbol) const;
        friend vstring operator+(char symbol, vstring& str);
 
        vstring& operator=(const char* s);
        vstring& operator=(const vstring& str);
 
        char* c_str();
    };

vstring.cpp

C++

#include "vstring.h"
 
unsigned int vstring::string_length(const char* str)
{
    if(str == 0) return 0;
 
    unsigned int size;
    for(size = 0; str[size] != '\0'; size++);
    return size;
}
 
char* vstring::string_copy(const char* str)
{
    if(str == 0)
    {
        char* ret = new char[1];
        ret[0] = '\0';
        return ret;
    }
 
    char* ret = new char[string_length(str)+1];
    ret[string_length(str)] = '\0';
    for(int i(0); i < string_length(str); i++)
        ret[i] = str[i];
    return ret;
}
 
vstring::vstring(const char* str)
{
    Str = string_copy(str);
    length = string_length(str);
}
vstring::vstring(char s)
{
    Str = new char[2];
    length = 1;
    Str[1] = '\0';
    Str[0] = s;
}
 
char& vstring::operator[](unsigned int index) const
{
    return Str[index];
}
 
vstring::vstring(const vstring& str)
{
    Str = string_copy(str.Str);
    length = str.length;
}
 
std::ostream& operator<<(std::ostream& out, const vstring& str) 
{
    for(int i(0); i < str.length; i++)
        out << str[i];
    return out;
}
 
vstring operator+(char symbol, vstring& str)
{
    return vstring(symbol) + str;
}
 
vstring vstring::operator+(const vstring& str) const
{
    char* resStr = new char[length + str.length + 1];
 
    int i(0);
    for(; i < length; i++)
        resStr[i] = Str[i];
    for(; i < length + str.length; i++)
        resStr[i] = str[i-length]; 
    resStr[i] = '\0';
        
        vstring ret(resStr);
        delete [] resStr;
    return ret;
}
 
vstring vstring::operator+(char symbol) const
{
    return *this + vstring(symbol);
}
 
vstring& vstring::operator=(const char* s)
{
    delete [] Str;
    Str = string_copy(s);
    length = string_length(s);
 
    return *this;
}
 
vstring& vstring::operator=(const vstring& str)
{
    delete [] Str;
 
    Str = string_copy(str.Str);
    length = str.length;
 
    return *this;
}
 
char* vstring::c_str()
{
    return string_copy(Str);
}

Сообщение от Croessmah

Также нет main'а, чтобы посмотреть как Вы всё это используете

Да пока никак не использую. Как раз и обратился, чтобы потом использовать без сомнений в работе класса.

Добавлено через 4 минуты

Сообщение от Croessmah

тогда можем попасть впросак,
если нет include guard'ов, т.к. прагмы не стандартны.

ну пусть подключит стандартные include guardы вместо прагмы.

Croessmah · 29.01.2016, 23:12

Сообщение от vaziliybober

ну пусть подключит стандартные include guardы вместо прагмы.

Это Ваша задача - Вы же разработчик этого инструмента

C++

char* vstring::c_str()
{
    return string_copy(Str);
}

Зачем что-то копировать и выделять?

C++

const char* vstring::c_str() const //объявление подправить соответственно. Добавить noexcept по возможности.
{
    return Str ;
}

@vaziliybober · 29.01.2016, 23:25 **[ТС]**

Сообщение от Croessmah

Зачем что-то копировать и выделять?

вот зачем

C++

vstring* str1 = new vstring("hello");
    char* str2 = str1->c_str();
    delete str1;
    cout << str2; // выдает крабозябры, т. к. удалился адрес Str (это если не копировать)

@hoggy · 29.01.2016, 23:34

Сообщение от vaziliybober

char& vstring::operator[](unsigned int index) const
{
* * return Str[index];
}

у вас метод константный.
это значит, что он работает в режиме "только для чтения".
но при этом он возвращает мутабельный символ.

вы уверены, что это правильно:
изменять данные, которые возвращают константные методы?

ведь технически, получается, что таким образом
можно изменить символ константной от рождения строки,
и нарваться на UB

Добавлено через 4 минуты

Сообщение от vaziliybober

вот зачем

vstring* str1 = new vstring("hello");
* * char* str2 = str1->c_str();
* * delete str1;
* * cout << str2; // выдает крабозябры, т. к. удалился адрес Str (это если не копировать)

привет, говнокод и утечки памяти.

@vaziliybober · 29.01.2016, 23:36 **[ТС]**

hoggy, я даже не знаю, что такое мутабельный символ. Никаких глубоких рассуждений у меня не было. Я просто не мог вызвать метод вот тут:

C++

std::ostream& operator<<(std::ostream& out, const vstring& str) 
{
    for(int i(0); i < str.length; i++)
        out << str[i]; // здесь
    return out;
}

Поэтому и приписал const.

Croessmah · 29.01.2016, 23:36

Сообщение от vaziliybober

вот зачем

а нефиг криво использовать.

@hoggy · 29.01.2016, 23:39

Сообщение от vaziliybober

Поэтому и приписал const.

замените:

C++

1 2	char& vstring::operator[](unsigned int index) const { return Str[index]; }

на:

C++

const char& vstring::operator[](unsigned int index) const
    { return Str[index]; }
 
char& vstring::operator[](unsigned int index) 
    { return Str[index]; }

либо на:

C++

1 2	char vstring::operator[](unsigned int index) const { return Str[index]; }

@vaziliybober · 29.01.2016, 23:47 **[ТС]**

hoggy, спасибо, исправил.

Сообщение от Croessmah

а нефиг криво использовать.

Но все же объясните 14-летнему чайнику-говнокодеру, как "ровно" использовать c_str(). Я ж просто хотел возможность преобразовывать в char*. Зачем мне в const char*?

@hoggy · 29.01.2016, 23:52

Сообщение от vaziliybober

как "ровно" использовать c_str().

вот жеж:

Сообщение от Croessmah

const char* vstring::c_str() const //объявление подправить соответственно. Добавить noexcept по возможности.
{
* * return Str ;
}

если объект-строка уничтожен,
значит данные строки тоже уничтожены.
значит любые указатели выданные наружу более не действительные.

об этом нужно знать и помнить,
когда вы снаружи кэшируете данные во внешних указателях.

@vaziliybober · 30.01.2016, 00:03 **[ТС]**

Сообщение от hoggy

об этом нужно знать и помнить,
когда вы снаружи кэшируете данные во внешних указателях.

ну а в конструкторах копирования мы ж без сомнений дублируем данные. Почему тогда в них нет таких заморочек?

@vaziliybober 1 / 1 / 0 Регистрация: 02.01.2016 Сообщений: 31
	29.01.2016, 23:47 [ТС]	18
	hoggy, спасибо, исправил. Сообщение от Croessmah а нефиг криво использовать. Но все же объясните 14-летнему чайнику-говнокодеру, как "ровно" использовать c_str(). Я ж просто хотел возможность преобразовывать в char. Зачем мне в const char? 0

@hoggy 8739 / 4317 / 960 Регистрация: 15.11.2014 Сообщений: 9,760
	29.01.2016, 23:52	19
	Сообщение от vaziliybober как "ровно" использовать c_str(). вот жеж: Сообщение от Croessmah const char* vstring::c_str() const //объявление подправить соответственно. Добавить noexcept по возможности. { * * return Str ; } если объект-строка уничтожен, значит данные строки тоже уничтожены. значит любые указатели выданные наружу более не действительные. об этом нужно знать и помнить, когда вы снаружи кэшируете данные во внешних указателях. 0

@Dimension Dimension 594 / 462 / 223 Регистрация: 08.04.2014 Сообщений: 1,710
	29.01.2016, 11:36	2
	это все уже есть в string , зачем свои велосипед делать не понимаю 0

@vaziliybober 1 / 1 / 0 Регистрация: 02.01.2016 Сообщений: 31
	29.01.2016, 11:43 [ТС]	3
	Dimension, это для школы, мы там только велосипеды и изобретаем. 0

@CheshireCat 2924 / 1274 / 114 Регистрация: 27.05.2008 Сообщений: 3,465
	29.01.2016, 11:49	5
	Ну, написание своих велосипедов - лучший способ обучения. Касательно самого кода: он у меня даже не компиляется (GCC 4.9), поэтому в дальнейшем анализе на "какие-то подводные камни, утечки памяти" не вижу смысла. 0

@vaziliybober 1 / 1 / 0 Регистрация: 02.01.2016 Сообщений: 31
	29.01.2016, 12:00 [ТС]	6
	Croessmah, спасибо, буду исправлять, видите, все же надо иногда учиться на велосипедах.CheshireCat, думаю, это из-за того vchar вместо char (описка) в строке 24 vstring.cpp 0

@CheshireCat 2924 / 1274 / 114 Регистрация: 27.05.2008 Сообщений: 3,465
	29.01.2016, 12:06	7
	Да, одна из ошибок была именно про vchar. Исправляй, как только код скомпиляется, погляжу более подробно и попробую написать несколько тестов, "ломающих" твой код. 0

Croessmah Неэпический 17870 / 10635 / 2054 Регистрация: 27.09.2012 Сообщений: 26,736 Записей в блоге: 1
	29.01.2016, 12:10	8
	Сообщение от vaziliybober Croessmah, спасибо, буду исправлять, видите, все же надо иногда учиться на велосипедах. Я этого не отрицал. Сам писал. 0

@vaziliybober 1 / 1 / 0 Регистрация: 02.01.2016 Сообщений: 31
	29.01.2016, 21:30 [ТС]	9
	Сообщение от CheshireCat Исправляй, как только код скомпиляется, погляжу более подробно и попробую написать несколько тестов, "ломающих" твой код. Если подключить <iostream> в vstring.h и "vstring.h" в vstring.cpp, то у меня на моем vs_2012 все компилится. Ну еще #pragma once можно на всякий. Мой косяк, не знаю, зачем я все это опустил. 0

Croessmah Неэпический 17870 / 10635 / 2054 Регистрация: 27.09.2012 Сообщений: 26,736 Записей в блоге: 1
	29.01.2016, 22:35	10
	Сообщение от vaziliybober Ну еще #pragma once можно на всякий тогда можем попасть впросак, если нет include guard'ов, т.к. прагмы не стандартны. Также нет main'а, чтобы посмотреть как Вы всё это используете 0

@hoggy 8739 / 4317 / 960 Регистрация: 15.11.2014 Сообщений: 9,760
	29.01.2016, 23:34	14
	Сообщение от vaziliybober char& vstring::operator[](unsigned int index) const { * * return Str[index]; } у вас метод константный. это значит, что он работает в режиме "только для чтения". но при этом он возвращает мутабельный символ. вы уверены, что это правильно: изменять данные, которые возвращают константные методы? ведь технически, получается, что таким образом можно изменить символ константной от рождения строки, и нарваться на UB Добавлено через 4 минуты Сообщение от vaziliybober вот зачем vstring* str1 = new vstring("hello"); * * char* str2 = str1->c_str(); * * delete str1; * * cout << str2; // выдает крабозябры, т. к. удалился адрес Str (это если не копировать) привет, говнокод и утечки памяти. 0

Croessmah Неэпический 17870 / 10635 / 2054 Регистрация: 27.09.2012 Сообщений: 26,736 Записей в блоге: 1
	29.01.2016, 23:36	16
	Сообщение от vaziliybober вот зачем а нефиг криво использовать. 0

@vaziliybober 1 / 1 / 0 Регистрация: 02.01.2016 Сообщений: 31
	30.01.2016, 00:03 [ТС]	20
	Сообщение от hoggy об этом нужно знать и помнить, когда вы снаружи кэшируете данные во внешних указателях. ну а в конструкторах копирования мы ж без сомнений дублируем данные. Почему тогда в них нет таких заморочек? 0

Написал свой string. Проверьте код на наличие подводных камней, утечек памяти и других ошибок

Решение