Ошибка деструктора - В конструкторе происходит выход за границу массива

@Retyrn0 · Регистрация: 24.06.2013

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

Доброго времени.

C++

#include <iostream.h>
#include <math.h>
 
class OKTAVE
{
public:
    OKTAVE(unsigned int Leng)
    {
        leng=Leng;
        wave=new signed short(leng);
        for(count=0;count<leng;count++)
        {
            wave[count]=(signed short)(32768*sin(count*3.1415927/leng));
            count++;
        }
        count=0;
    }
    ~OKTAVE(){delete[]wave;}
 
private:
    signed short*wave;
    unsigned short count,leng;
};
 
int main()
{
    OKTAVE okt(1024);
 
    return 0;
}

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

Добавлено через 1 минуту
При удалении экземпляра класса вылетает ошибка

@nmcf · 05.07.2014, 21:22

14-ю строку убери.

@Retyrn0 · 05.07.2014, 21:35 **[ТС]**

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

14-ю строку убери.

Туплю =)
Спасибо.

Добавлено через 48 секунд
Не помогло.

Добавлено через 1 минуту
Всё та же ошибка вылетает.

Добавлено через 1 минуту
Надо воздухом подышать...перепутал [] c (). Тема закрыта.

Tulosba · 06.07.2014, 11:03

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

C++

1	#include <iostream.h>

Пора подумать о замене компилятора.
Дополнительно:
Вместо ручного выделения памяти можно рассмотреть std::vector. Это позволит например избавиться от деструктора.
Странно использовать под переменную цикла поле класса.
short по умолчанию и так signed.
Для числа Пи есть константа M_PI
Для граничных значений типа T имеет смысл использовать std::numeric_limits<T>

@Retyrn0 · 06.07.2014, 13:23 **[ТС]**

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

Пора подумать о замене компилятора.

Новые версии долговато компилируют =/

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

Странно использовать под переменную цикла поле класса.

count выполняет в классе важную роль - я не все методы копировал сюда...маничка такая у меня - не люблю создавать переменные, если есть уже созданные, не используемые(она используется после инициализации в др. методе.)

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

Вместо ручного выделения памяти можно рассмотреть std::vector. Это позволит например избавиться от деструктора.

Массив выделяется единожды и многообразие возможностей вектора здесь излишни. Пардон за сленг, но мне не впадлу написать целую строчку в деструкторе

Но спасибо за замечания, гляну что можно сделать.

Tulosba · 06.07.2014, 14:07

Retyrn0, новые версии - это в первую очередь новые возможности и исправление ошибок и недочетов (с внесением новых конечно же

). Вообще, когда начинается разговор о скорости, надо приводить конкретные тесты, иначе всё это пустое. К тому же удивительно, что Вы замечаете разницу в скорости на тривиальных программах.

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

не люблю создавать переменные, если есть уже созданные, не используемые

Использовать одинаковые переменные для разных целей - не лучший вариант, т.к. смахивает на преждевременную оптимизацию. А оптимизация, как известно, в первую очередь негативно сказывается на понимании кода.
Хотя случай со счетчиком немного из другой оперы, и выносить его в члены класса не стоит, так он не имеет отношения к состоянию объекта, но при этом увеличивает его размер.

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

Массив выделяется единожды

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

@Retyrn0 · 06.07.2014, 14:20 **[ТС]**

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

одинаковые переменные для разных целей

цель соответствует названию - счётчик. Он и в конструкторе и в методах считает текущую позицию в массиве, только в конструкторе для инициализации.

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

Хотя случай со счетчиком немного из другой оперы, и выносить его в члены класса не стоит, так он не имеет отношения к состоянию объекта, но при этом увеличивает его размер.

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

count выполняет в классе важную роль

Он используется классом и напрямую имеет отношение к состоянию объекта. Просто здесь не все методы, иначе было бы понятно.

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

отказаться от динамического выделения памяти

Размер массива в разных экземплярах класса разный - без динамического выделения никуда.
Мне нужно в начале выделить, в конце уничтожить - всё. Это 2 строчки, здесь сложно наделать ошибок. И на утечки я проверяю, так что с этим у меня всегда был порядок)

Tulosba · 06.07.2014, 14:51

Retyrn0, если счетчик имеет отношение к состоянию объекта, то он не должен использоваться как переменная цикла. Так что определиться все таки не помешает для чего же он предназначен. Вы используете значение счетчика в каком-нибудь методе после установки его в другом? Можете показать пример?

И на утечки я проверяю, так что с этим у меня всегда был порядок

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

Добавлено через 1 минуту
На счет разного размера массива - можно и шаблон класса сделать с размером в качестве аргумента. Без динамического выделения.

@Retyrn0 · 06.07.2014, 15:09 **[ТС]**

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

если счетчик имеет отношение к состоянию объекта, то он не должен использоваться как переменная цикла. Так что определиться все таки не помешает для чего же он предназначен. Вы используете значение счетчика в каком-нибудь методе после установки его в другом? Можете показать пример?

Метод конструктора запускается единожды, при инициализации, и пока не выполнилась инициализация, остальные методы не могут запуститься, так что одновременный доступ исключён. В конструкторе в конце есть

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

count=0;

Т.е. конструктор после выполнения необходимых вертикулей обнуляет его и больше НИКОГДА не использует, соответственно до завершения работы конструктора никакой метод, использующий count не может быть запущен - здесь полный порядок, не переживайте)

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

Слишком самоуверенно.

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

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

На счет разного размера массива - можно и шаблон класса сделать с размером в качестве аргумента. Без динамического выделения.

Зачем? Всё же и так работает правильно. К чему усложнять? Всего-то 2 строчки с new[] и delete[]. Зачем шаблоны класса и пр? Не исключаю, что в другой задаче я бы согласился(где сложно определить когда выделять, когда удалять память), но здесь всё лаконично и понятно.

@DrOffset · 06.07.2014, 15:53

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

маничка такая у меня - не люблю создавать переменные, если есть уже созданные

Как человек, который тоже этим в далеком прошлом переболел, советую от этой мании избавляться. Во-первых память на стеке выделяется очень быстро. Ты ничего не выиграешь на этом. Особенно на современных процессорах (почитай про конвейеры и кеш).
Во-вторых, в будущем при профессиональной разработке в команде могут быть проблемы с сопровождением такого кода другими людьми. Увидит человек переменную-счетчик, а определения рядом нет. Значит какой вывод? Правильно, она где-то еще используется, и возможно является частью какого-то нетривиального алгоритма (ну он же не знает про твой хитрый план экономии переменных). И вот полезет он шерстить все методы, где еще эта переменная используется, потратит время, поругается. В целом все это довольно неприятно. Есть такой принцип - "принцип наименьшего удивления". В твоем маленьком коде это не очевидно, но когда проект на пару сотен тысяч строк - это очень серьезный фактор, когда что-то сделано не как обычно.

Tulosba · 06.07.2014, 15:59

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

никакой метод, использующий count

Не надо лить воду про конструктор - покажите лучше этот метод.

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

а понимаю что происходит с байтами

И что же с ними происходит?

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

но здесь всё лаконично и понятно.

Какой вариант лаконичнее:

C++

class A
{
public:
    A(int size) { data = new int[size]; }
    ~A() { delete [] data; }
private:
    int* data;
};

или

C++

class B
{
public:
    B(int size) : data(size) { }
private:
    std::vector<int> data;
};

?

@Retyrn0 · 06.07.2014, 16:22 **[ТС]**

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

Во-первых память на стеке выделяется очень быстро

Я не о скорости...
Как же объяснить то...по вашему я должен быть создать внутри конструктора переменную count1 ну или count? А зачем, если она и так есть и пока свободна?

Tulosba · 06.07.2014, 16:28

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

по вашему я должен быть создать внутри конструктора переменную count1 ну или count?

Для цикла следовало бы задать свою переменную:

C++

1	for(int i=0; i<leng; i++) ...

@Retyrn0 · 06.07.2014, 16:28 **[ТС]**

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

покажите лучше этот метод

C++

signed short SetSempl(signed short sempl)
{
   wave[count]=sempl;
   count++;
   if(count>len)count=0;
}

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

Какой вариант лаконичнее:

У меня лично первый вариант не вызывает никаких проблем.

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

И что же с ними происходит?

Веселье там, жизнь ХД

Tulosba · 06.07.2014, 16:31

Retyrn0, не логичнее было бы сделать так?

C++

short SetSempl(short sempl, int index)
{
   // здесь проверить index на диапазон
   wave[index]=sempl;
}

@Retyrn0 · 06.07.2014, 16:34 **[ТС]**

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

Для цикла следовало бы задать свою переменную

Да что Вы придрались к моей переменной? Пусть себе считает)

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

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

не логичнее было бы сделать так?

Видите ли, каждый экземпляр класса имеет разную длину. Что я им должен передавать в качестве index?

@DrOffset · 06.07.2014, 16:34

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

по вашему я должен быть создать внутри конструктора переменную count1 ну или count?

Нет. Т.к. ты не выложил или не рассказал на словах зачем нужна переменная count в классе, то выводы пока делать рано.

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

А зачем, если она и так есть и пока свободна?

Это зависит от многих факторов, о которых ты не рассказал. Но я говорил не про этот конкретный случай, а в целом. Т.е. у каждой переменной должна быть своя конкретная задача, свое предназначение. Если переменная по совместительству и швец и жнец, то это порождает проблемы, о которых я писал выше.

@Retyrn0 · 06.07.2014, 16:38 **[ТС]**

Получается, что я должен отдельно от класса создать массив индексов, обрабатывать их в основном цикле? Но зачем мне тогда вообще нужен будет класс? - Создать сразу и массивы, как не объектные переменные и радоваться жизни

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

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

о которых ты не рассказал

Я же сразу намекнул, что я рассказал не всё и что там порядок.

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

по совместительству и швец и жнец

У меня она по совместительству и швец и швец) А вот зачем мне два швеца не знаю)

@DrOffset · 06.07.2014, 16:41

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

что я рассказал не всё и что там порядок.

Да даже дядькам с 20+ лет опыта не стоит верить про порядок

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

У меня она по совместительству и швец и швец) А вот зачем мне два швеца не знаю)

Вот на что я точно не надеялся, что ты сразу же переменишь свое мнение. Остальное к опытом придет.

Tulosba · 06.07.2014, 16:44

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

Да что Вы придрались к моей переменной? Пусть себе считает)

Я просто даю советы, которые считаю правильными. Прислушиваться к ним или нет - дело сугубо личное.
Однако, есть общепризнанные практики и правила хорошего тона программирования. Если их не придерживаться, будет трудно контролировать код в процессе его развития.

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 16:22 [ТС]	12
	Сообщение от DrOffset Во-первых память на стеке выделяется очень быстро Я не о скорости... Как же объяснить то...по вашему я должен быть создать внутри конструктора переменную count1 ну или count? А зачем, если она и так есть и пока свободна? 0

@DrOffset 18896 / 9854 / 2410 Регистрация: 30.01.2014 Сообщений: 17,297
	06.07.2014, 16:34	17
	Сообщение от Retyrn0 по вашему я должен быть создать внутри конструктора переменную count1 ну или count? Нет. Т.к. ты не выложил или не рассказал на словах зачем нужна переменная count в классе, то выводы пока делать рано. Сообщение от Retyrn0 А зачем, если она и так есть и пока свободна? Это зависит от многих факторов, о которых ты не рассказал. Но я говорил не про этот конкретный случай, а в целом. Т.е. у каждой переменной должна быть своя конкретная задача, свое предназначение. Если переменная по совместительству и швец и жнец, то это порождает проблемы, о которых я писал выше. 0

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 16:38 [ТС]	18
	Получается, что я должен отдельно от класса создать массив индексов, обрабатывать их в основном цикле? Но зачем мне тогда вообще нужен будет класс? - Создать сразу и массивы, как не объектные переменные и радоваться жизни Добавлено через 1 минуту Сообщение от DrOffset о которых ты не рассказал Я же сразу намекнул, что я рассказал не всё и что там порядок. Сообщение от DrOffset по совместительству и швец и жнец У меня она по совместительству и швец и швец) А вот зачем мне два швеца не знаю) 0

@DrOffset 18896 / 9854 / 2410 Регистрация: 30.01.2014 Сообщений: 17,297
	06.07.2014, 16:41	19
	Сообщение от Retyrn0 что я рассказал не всё и что там порядок. Да даже дядькам с 20+ лет опыта не стоит верить про порядок Сообщение от Retyrn0 У меня она по совместительству и швец и швец) А вот зачем мне два швеца не знаю) Вот на что я точно не надеялся, что ты сразу же переменишь свое мнение. Остальное к опытом придет. 0

@nmcf 7795 / 6561 / 2985 Регистрация: 14.04.2014 Сообщений: 28,683
	05.07.2014, 21:22	2
	14-ю строку убери. 1

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	05.07.2014, 21:35 [ТС]	3
	Сообщение от nmcf 14-ю строку убери. Туплю =) Спасибо. Добавлено через 48 секунд Не помогло. Добавлено через 1 минуту Всё та же ошибка вылетает. Добавлено через 1 минуту Надо воздухом подышать...перепутал [] c (). Тема закрыта. 0

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 13:23 [ТС]	5
	Сообщение от Tulosba Пора подумать о замене компилятора. Новые версии долговато компилируют =/ Сообщение от Tulosba Странно использовать под переменную цикла поле класса. count выполняет в классе важную роль - я не все методы копировал сюда...маничка такая у меня - не люблю создавать переменные, если есть уже созданные, не используемые(она используется после инициализации в др. методе.) Сообщение от Tulosba Вместо ручного выделения памяти можно рассмотреть std::vector. Это позволит например избавиться от деструктора. Массив выделяется единожды и многообразие возможностей вектора здесь излишни. Пардон за сленг, но мне не впадлу написать целую строчку в деструкторе Но спасибо за замечания, гляну что можно сделать. 0

Tulosba :) 4773 / 3267 / 497 Регистрация: 19.02.2013 Сообщений: 9,046
	06.07.2014, 14:07	6
	Retyrn0, новые версии - это в первую очередь новые возможности и исправление ошибок и недочетов (с внесением новых конечно же). Вообще, когда начинается разговор о скорости, надо приводить конкретные тесты, иначе всё это пустое. К тому же удивительно, что Вы замечаете разницу в скорости на тривиальных программах. Сообщение от Retyrn0 не люблю создавать переменные, если есть уже созданные, не используемые Использовать одинаковые переменные для разных целей - не лучший вариант, т.к. смахивает на преждевременную оптимизацию. А оптимизация, как известно, в первую очередь негативно сказывается на понимании кода. Хотя случай со счетчиком немного из другой оперы, и выносить его в члены класса не стоит, так он не имеет отношения к состоянию объекта, но при этом увеличивает его размер. Сообщение от Retyrn0 Массив выделяется единожды Так и вектор выделит память однократно при правильном использовании. Меньше ручного управления памятью - меньше ошибок. Возможно Вам вообще стоило бы тогда отказаться от динамического выделения памяти, если пугает потенциальный оверхед от использования вектора. 0

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 14:20 [ТС]	7
	Сообщение от Tulosba одинаковые переменные для разных целей цель соответствует названию - счётчик. Он и в конструкторе и в методах считает текущую позицию в массиве, только в конструкторе для инициализации. Сообщение от Tulosba Хотя случай со счетчиком немного из другой оперы, и выносить его в члены класса не стоит, так он не имеет отношения к состоянию объекта, но при этом увеличивает его размер. Сообщение от Retyrn0 count выполняет в классе важную роль Он используется классом и напрямую имеет отношение к состоянию объекта. Просто здесь не все методы, иначе было бы понятно. Сообщение от Tulosba отказаться от динамического выделения памяти Размер массива в разных экземплярах класса разный - без динамического выделения никуда. Мне нужно в начале выделить, в конце уничтожить - всё. Это 2 строчки, здесь сложно наделать ошибок. И на утечки я проверяю, так что с этим у меня всегда был порядок) 0

Tulosba :) 4773 / 3267 / 497 Регистрация: 19.02.2013 Сообщений: 9,046
	06.07.2014, 14:51	8
	Retyrn0, если счетчик имеет отношение к состоянию объекта, то он не должен использоваться как переменная цикла. Так что определиться все таки не помешает для чего же он предназначен. Вы используете значение счетчика в каком-нибудь методе после установки его в другом? Можете показать пример? И на утечки я проверяю, так что с этим у меня всегда был порядок Слишком самоуверенно. Будь оно все так легко и просто, классы "умных" указателей вообще бы никогда не появились, т.к. не были бы востребованы. Добавлено через 1 минуту На счет разного размера массива - можно и шаблон класса сделать с размером в качестве аргумента. Без динамического выделения. 0

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 15:09 [ТС]	9
	Сообщение от Tulosba если счетчик имеет отношение к состоянию объекта, то он не должен использоваться как переменная цикла. Так что определиться все таки не помешает для чего же он предназначен. Вы используете значение счетчика в каком-нибудь методе после установки его в другом? Можете показать пример? Метод конструктора запускается единожды, при инициализации, и пока не выполнилась инициализация, остальные методы не могут запуститься, так что одновременный доступ исключён. В конструкторе в конце есть Сообщение от Retyrn0 count=0; Т.е. конструктор после выполнения необходимых вертикулей обнуляет его и больше НИКОГДА не использует, соответственно до завершения работы конструктора никакой метод, использующий count не может быть запущен - здесь полный порядок, не переживайте) Сообщение от Tulosba Слишком самоуверенно. Я привык к низкоуровневости - я не просто знаю что после чего нужно писать, а понимаю что происходит с байтами. А классы умных указателей придумали не потому что не возможно грамотно работать с памятью, а скорее для удобства. Сообщение от Tulosba На счет разного размера массива - можно и шаблон класса сделать с размером в качестве аргумента. Без динамического выделения. Зачем? Всё же и так работает правильно. К чему усложнять? Всего-то 2 строчки с new[] и delete[]. Зачем шаблоны класса и пр? Не исключаю, что в другой задаче я бы согласился(где сложно определить когда выделять, когда удалять память), но здесь всё лаконично и понятно. 0

@DrOffset 18896 / 9854 / 2410 Регистрация: 30.01.2014 Сообщений: 17,297
	06.07.2014, 15:53	10
	Сообщение от Retyrn0 маничка такая у меня - не люблю создавать переменные, если есть уже созданные Как человек, который тоже этим в далеком прошлом переболел, советую от этой мании избавляться. Во-первых память на стеке выделяется очень быстро. Ты ничего не выиграешь на этом. Особенно на современных процессорах (почитай про конвейеры и кеш). Во-вторых, в будущем при профессиональной разработке в команде могут быть проблемы с сопровождением такого кода другими людьми. Увидит человек переменную-счетчик, а определения рядом нет. Значит какой вывод? Правильно, она где-то еще используется, и возможно является частью какого-то нетривиального алгоритма (ну он же не знает про твой хитрый план экономии переменных). И вот полезет он шерстить все методы, где еще эта переменная используется, потратит время, поругается. В целом все это довольно неприятно. Есть такой принцип - "принцип наименьшего удивления". В твоем маленьком коде это не очевидно, но когда проект на пару сотен тысяч строк - это очень серьезный фактор, когда что-то сделано не как обычно. 0

@Retyrn0 45 / 48 / 5 Регистрация: 24.06.2013 Сообщений: 677
	06.07.2014, 16:34 [ТС]	16
	Сообщение от Tulosba Для цикла следовало бы задать свою переменную Да что Вы придрались к моей переменной? Пусть себе считает) Добавлено через 2 минуты Сообщение от Tulosba не логичнее было бы сделать так? Видите ли, каждый экземпляр класса имеет разную длину. Что я им должен передавать в качестве index? 0

Tulosba :) 4773 / 3267 / 497 Регистрация: 19.02.2013 Сообщений: 9,046
	06.07.2014, 16:44	20
	Сообщение от Retyrn0 Да что Вы придрались к моей переменной? Пусть себе считает) Я просто даю советы, которые считаю правильными. Прислушиваться к ним или нет - дело сугубо личное. Однако, есть общепризнанные практики и правила хорошего тона программирования. Если их не придерживаться, будет трудно контролировать код в процессе его развития. 0