Форум программистов, компьютерный форум CyberForum.ru

Просьба оценить код - C++

Восстановить пароль Регистрация
 
Ksan
26 / 26 / 0
Регистрация: 02.11.2010
Сообщений: 370
28.06.2012, 19:36     Просьба оценить код #1
Данный код реализует массивы, размеры которых можно легко изменять, а так же которые можно легко склеивать. Прошу оценить его и покритиковать.


Element.h - Отвечает за элементы массива
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
template <typename T> class Element
{
        public:
                Element()
                {
                        _val = 0;
                        _next = NULL;
                }
                Element(T val)
                {
                        _val = val;
                        _next = NULL;
                }
                Element(T val, Element<T> &next)
                {
                        _val = val;
                        _next = &next;
                }
                void operator=(T val)
                {
                        _val = val;
                }
                Element<T>& operator>>(Element<T> &next)
                {
                        _next = &next;
                        return next;
                }
                Element<T>* operator>>(Element<T> *next)
                {
                        _next = next;
                        return next;
                }
                T& val(unsigned int index)
                {
                        if(index == 0) return _val;
                        
                        Element<T> *el = _next;
                        while(--index > 0) el = el->_next;
                        
                        return el->_val;
                }
                Element<T>* elm(unsigned int index)
                {
                        Element<T> *el = _next;
                        while(--index > 0) el = el->_next;
                        
                        return el;
                }
        private:
                Element<T> *_next;
                T _val;
};

Array.h - Отвечает за сам массив
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
template <typename T> class Array
{
        public:
                Array() 
                {
                        _len = 0;
                }
                Array(unsigned int len)
                {
                        _len = len;
                        
                        if(len-- == 0) return ; // if(len == 0) return ;
                        if(len-- == 0) return ; // if(len == 1) return ;
                        
                        Element<T> *el = new Element<T>;
                        _array >> el;
                        
                        if(len == 0) return ; // if(len == 2) return ;
                        
                        Element<T> *el2;
                        
                        for( ; len>0; --len)
                        {
                                el2 = new Element<T>;
                                (*el) >> el2;
                                el = el2;
                        }
                }
                ~Array()
                {
                        for(--_len; _len>0; --_len)
                        {
                                delete _array.elm(_len);
                        }
                }
                T& operator[](unsigned int index)
                {
                        return _array.val(index);
                }
                void operator>>(unsigned int len)
                {
                        if(len == 0) return ;
                        Element<T> *el, *el2;
                        
                        if(_len == 0 || _len == 1)
                        {
                                el = &_array;
                        } else {
                                el = _array.elm(_len - 1);
                        }
                        
                        _len += len;
                        
                        for( ; len>0; --len)
                        {
                                el2 = new Element<T>;
                                (*el) >> el2;
                                el = el2;
                        }
                }
                void operator<<(unsigned int len)
                {
                        for( ; len>0; --len)
                        {
                                delete _array.elm(--_len);
                        }
                }
                void operator<<(Array<T> &array)
                {
                        *(_array.elm(_len - 1)) >> array._array;
                        _len += array._len;
                }
                unsigned int length()
                {
                        return _len;
                }
        private:
                unsigned int _len;
                Element<T> _array; 
};

Main.cpp - Пример работы с массивом
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
#include <iostream>
using namespace std;
 
#include "Element.h"
#include "Array.h"
 
int main()
{
        Array<int> arr(5); // ñîçäГ*ГҐГІ Г¬Г*Г±Г±ГЁГў Г± 5Гѕ ГїГ·ГҐГЄГ*ìè [0..4];
            
        arr >> 2; // äîáГ*âëÿåò  2 ýëåìåГ*ГІГ* ГЄ Г¬Г*Г±Г±ГЁГўГі
        arr << 3; // ГіГ*è÷òîæГ*ГҐГІ 3 ïîñëåäГ*ГЁГµ ýëåìåГ*ГІГ* Г¬Г*Г±Г±ГЁГўГ*  
        for(int i=0; i<arr.length(); ++i) arr[i] = i+1; // Г§Г*ïîëГ*ГїГҐГІ Г¬Г*Г±Г±ГЁГў
        
        Array<int> arr_2(3);
        arr_2 << arr; // ïðèêëåèâГ*ГҐГІ Г¬Г*Г±Г±ГЁГў arr ГЄ ГЄГ®Г*öó Г¬Г*Г±Г±ГЁГўГ* arr_2
        
        for(int i=0; i<arr_2.length(); ++i)
        {
                cout << arr_2[i] << endl;
        }   
            
        while(1);
        return 0;
}



ЗЫ: да, я писал реализацию функций внутри класса, потому что для такого небольшого кода смысла выносить нету.
Similar
Эксперт
41792 / 34177 / 6122
Регистрация: 12.04.2006
Сообщений: 57,940
28.06.2012, 19:36     Просьба оценить код
Посмотрите здесь:

C++ Просьба к администрации
Убедительная просьба помочь... C++
C++ Глупая просьба...
Просьба с компиляцией C++
Просьба C++
После регистрации реклама в сообщениях будет скрыта и будут доступны все возможности форума.
soon
 Аватар для soon
2536 / 1301 / 81
Регистрация: 09.05.2011
Сообщений: 3,086
Записей в блоге: 1
28.06.2012, 20:45     Просьба оценить код #2
Цитата Сообщение от Ksan Посмотреть сообщение
C++
1
2
3
4
5
Element()
{
    _val = 0;
    _next = NULL;
}
Для кого были придуманы списки инициализации?
C++
1
Element(T val)
А если T это класс, в котором при вызове конструктора создается вектор на стотыщмильенэлементов?

Цитата Сообщение от Ksan Посмотреть сообщение
C++
1
while(1);
Это таким образом вы боретесь с сегфолтом?

Дальше не смотрел. Исходя из вышесказанного, считаю нужным устранить эти недочеты, чтобы программа хотя бы нормально работала. Кстати, особо не вчитывался, но что если сделать так arr << arr?

ps/ NULL определен в cstdlib. Я уж молчу про новый стандарт.
ЛетающийЕнот
88 / 67 / 12
Регистрация: 28.06.2012
Сообщений: 161
28.06.2012, 20:49     Просьба оценить код #3
Ksan,
И этим я своё отношение выразил.

Недочёты есть. Серьёзные, но своей цели ты достиг.
Ksan
26 / 26 / 0
Регистрация: 02.11.2010
Сообщений: 370
28.06.2012, 20:54  [ТС]     Просьба оценить код #4
Цитата Сообщение от soon Посмотреть сообщение
Для кого были придуманы списки инициализации?
Что, позвольте?


Цитата Сообщение от soon Посмотреть сообщение
А если T это класс, в котором при вызове конструктора создается вектор на стотыщмильенэлементов?
В первую очередь я ориентировался на стандартные типы: int, short, float, char etc
Можно доделать и с расчетом на такие классы


Цитата Сообщение от soon Посмотреть сообщение
Это таким образом вы боретесь с сегфолтом?
Это для того, что бы консоль не закрылась и можно было посмотреть выведенный текст


Цитата Сообщение от soon Посмотреть сообщение
Я уж молчу про новый стандарт.
И что говорит новый стандарт?



Цитата Сообщение от soon Посмотреть сообщение
что если сделать так arr << arr?
Склеится нормально. Но возникнет интересный эффект: при изменении одного элемента массива будет меняться еще один

Добавлено через 33 секунды
ЛетающийЕнот, а какие недочеты?
что бы на будущее знать
ЛетающийЕнот
88 / 67 / 12
Регистрация: 28.06.2012
Сообщений: 161
28.06.2012, 21:03     Просьба оценить код #5
Цитата Сообщение от Ksan Посмотреть сообщение
ЛетающийЕнот, а какие недочеты?
Боюсь, что большинство уже указал уважаемый soon.

Если задуматься об оптимизации, то я бы кое-что поправил существенно.
Это можно с одной стороны считать не столь важным: ты хотел освоить шаблоны, у тебя получился довольный код.
С другой стороны, про подобное тоже забывать нельзя.
soon
 Аватар для soon
2536 / 1301 / 81
Регистрация: 09.05.2011
Сообщений: 3,086
Записей в блоге: 1
28.06.2012, 21:13     Просьба оценить код #6
Цитата Сообщение от Ksan Посмотреть сообщение
Что, позвольте?
Оччень интересно, изучаете уже шаблоны а про списки инициализации впервые слышите. Загуглите, что ли.
Цитата Сообщение от Ksan Посмотреть сообщение
Можно доделать и с расчетом на такие классы
Нужно так делать изначально. Это приходит со временем, так что сосредоточтесь пока на остальных недочетах.
Цитата Сообщение от Ksan Посмотреть сообщение
Это для того, что бы консоль не закрылась и можно было посмотреть выведенный текст
Я про
это
Bash
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
soon@desktop:~/Src/C++/test$ make && ./test
g++ -c -Wall -O3 --std=gnu++11 foo.cpp -o foo.o
g++ -c -Wall -O3 --std=gnu++11 test.cpp -o test.o
test.cpp: In function ‘int main()’:
test.cpp:151:35: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
test.cpp:156:37: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
g++ foo.o test.o -lm -o test
0
0
0
1
2
3
4
Segmentation fault (core dumped)
soon@desktop:~/Src/C++/test$

А про закрыте консоли говорилось и не раз: консольные программы надо запускать из под консоли. И тогда не нужны будут всякие костыли.
Цитата Сообщение от Ksan Посмотреть сообщение
И что говорит новый стандарт?
Использовать nullptr и rvalue reference. А использование NULL осуждается в C++ уже давно. Вместо него принято использовать 0.
Цитата Сообщение от Ksan Посмотреть сообщение
Склеится нормально. Но возникнет интересный эффект: при изменении одного элемента массива будет меняться еще один
Не есть хорошо, вам не кажется? Может я хочу просто продублировать элементы. Оставим на ваше усмотрение, но мне бы пришлась по душе такая возможность.

В общем, жду переработанный код.
ForEveR
Модератор
Эксперт C++
 Аватар для ForEveR
7927 / 4709 / 318
Регистрация: 24.06.2010
Сообщений: 10,524
Завершенные тесты: 3
28.06.2012, 22:59     Просьба оценить код #7
Ну откровенно говоря, перегрузка операторов крайне желательна должна соответствовать тому что принято... arr >> 2 - добавить 2 элемента, вообще ни разу не очевидно.
Ksan
26 / 26 / 0
Регистрация: 02.11.2010
Сообщений: 370
28.06.2012, 23:24  [ТС]     Просьба оценить код #8
ForEveR, а как вы бы их перегрузили?
Jupiter
Каратель
Эксперт C++
6542 / 3962 / 226
Регистрация: 26.03.2010
Сообщений: 9,273
Записей в блоге: 1
Завершенные тесты: 2
29.06.2012, 00:23     Просьба оценить код #9
Цитата Сообщение от Ksan Посмотреть сообщение
Данный код реализует массивы
это вообще не массив, а односвязный список, массив подразумевает последовательное хранение элементов т.е. в памяти элементы должны лежать один за другим
Ksan
26 / 26 / 0
Регистрация: 02.11.2010
Сообщений: 370
29.06.2012, 00:24  [ТС]     Просьба оценить код #10
Jupiter, хорошо. Назвал неправильно
ForEveR
Модератор
Эксперт C++
 Аватар для ForEveR
7927 / 4709 / 318
Регистрация: 24.06.2010
Сообщений: 10,524
Завершенные тесты: 3
29.06.2012, 00:40     Просьба оценить код #11
Ksan, Я бы их если и перегружал, то только как оператор ввода/вывода в поток, ибо сдвиг для этого класса как бэ не нужен.
DU
1477 / 1053 / 45
Регистрация: 05.12.2011
Сообщений: 2,279
29.06.2012, 00:43     Просьба оценить код #12
Нет конструктора копирования и оператора =. Двойное удаление + утечки из-за этого будут.
Константные по смыслу методы должны быть константными. Например length.
Нет константной вервии оператора [].
Всем операциям, которые в операторах << и >> лучше дать нормальные имена. с операторами сложно понимать, что происходит.
Присоединение массива кривое. После этой операции два массива будут владеть ресурсами. При их разрушении будет двойное удаление элементов.
Jupiter
29.06.2012, 00:53
  #13

Не по теме:

Цитата Сообщение от ForEveR Посмотреть сообщение
ибо сдвиг для этого класса как бэ не нужен.
почему же, очень даже нужен, но только один
containter << elem1 << elem2 << elem3
мелочь, а удобно

DU
1477 / 1053 / 45
Регистрация: 05.12.2011
Сообщений: 2,279
29.06.2012, 00:54     Просьба оценить код #14
Код в Array.h какой-то мудренный, и с копипастом. Хотя можно было бы использовать методы. Например создание массива длинной n за счет функции добавления в массив элемента.
Полезность класса Element сомнительна.
MoreAnswers
Эксперт
37091 / 29110 / 5898
Регистрация: 17.06.2006
Сообщений: 43,301
29.06.2012, 00:54     Просьба оценить код
Еще ссылки по теме:

просьба объяснить C++
просьба просто скомпилить и запустить код C++
C++ Просьба исправить ошибки

Искать еще темы с ответами

Или воспользуйтесь поиском по форуму:
ForEveR
29.06.2012, 00:54     Просьба оценить код
  #15

Не по теме:

Jupiter, Ну это не сдвиг (я имел ввиду битовый сдвиг), вообщем-то удобно конечно, но в бусте наверное не зря реализовали добавление элементов через +=, а не через <<. А сейчас это вообще не актуально уже, достаточно сделать container.insert(container.end(), {elem1,elem2,elem3});

Yandex
Объявления
29.06.2012, 00:54     Просьба оценить код
Ответ Создать тему
Опции темы

Текущее время: 06:52. Часовой пояс GMT +3.
КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin® Version 3.8.9
Copyright ©2000 - 2016, vBulletin Solutions, Inc.
Рейтинг@Mail.ru