Форум программистов, компьютерный форум, киберфорум
Наши страницы

С++ для начинающих

Войти
Регистрация
Восстановить пароль
 
 
Рейтинг: Рейтинг темы: голосов - 17, средняя оценка - 4.71
metaluga145
243 / 244 / 20
Регистрация: 08.04.2013
Сообщений: 927
#1

Критику в студию - C++

25.04.2013, 23:39. Просмотров 2476. Ответов 30
Метки нет (Все метки)

Доброго всем времени суток! Я вот написал такой себе класс матриц, прошу оценить его и посоветовать что можно улучшить или изменить, как повысить быстродействие или, возможно, что-то надо добавить.
Знаю,что надо сделать шаблон класса для разных типов данных, но дабы сейчас все было красиво и хорошо не загрязнял этим код! Любые дельные советы будут "лайкнуты"!

matrix.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
//класс матриц
//matrix.h
/*проверка от дураков не предусмотрена*/
#pragma once
#include <iostream>
#include <fstream>
 
class matrix
{
public:
    matrix();//конструктор по умолчанию
    matrix(const int& , const int&);//конструктор, создает еденичную матрицу
    matrix(const int& , const int& , const double *array);//конструктор, берет значения из массива
    matrix::matrix(matrix &);//конструктор копирования
    ~matrix();//деструктор
 
protected:
    double *array;//массив матрицы
    int rows, columns;//количество столбцов и строк
    int precision;//точность вывода матрицы
 
public:
//операции с матрицей
    matrix& set(const double* );//функция изминения значения всех элементов матрицы
    matrix& eye();//делает матрицу еденичной(возможно не квадратной)
    matrix& zero();//делает матрицу нулевой
    matrix  transpose();//функция траспонирования, возвращает матрицу
//операторы
    matrix  operator*(const matrix& );//оператор умножения матриц
    matrix  operator*(const double&);//оператор умножение на дабл
    matrix  operator+(const matrix& );//оператор сложения матриц
    matrix  operator-(const matrix& );//оператор вычитания матриц
    matrix& operator=(const matrix& );//оператор присваивания
    double&      operator()(const int&, const int&);//оператор получения элемента матрицы
//дополнительные функции
    void householderDecomposition(matrix& Q, matrix& R);//алгоритм Хаусхолдера, изменяет параметры-ссылки
//функции установок параметров матрицы
    void output();//вывод матрицы на экран
    void output(std::ofstream* );//вывод матрицы в файл
    void SetSize (const int&, const int&);//изменение размеров
    int& Columns (void);//вывод количества столбцов
    int& Rows (void);//вывод количества строк
    void SetPrecison (const int &);//установка точности вывода
};
matrix.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
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
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
//класс матриц
//matrix.cpp
/*проверка от дураков не предусмотрена*/
#include "StdAfx.h"
#include "matrix.h"
#include <iomanip>
#include <math.h>
 
//конструктор по умолчанию
matrix::matrix():rows(0),columns(0),precision(2)
{
    array=NULL;
}
//конструктор еденичной матрицы
matrix::matrix(const int& rows, const int& columns) :
    rows(rows), columns(columns), precision(2)
{
    array = new double[rows * columns];
    this->eye();
}
//конструктор из массива
matrix::matrix(const int& rows, const int& columns, const double *array) :
    rows(rows), columns(columns), precision(2)
{
    this->array = new double[rows * columns];
    for (int i = 0; i < rows * columns; i++) this->array[i] = array[i];
}
//конструктор копирования
matrix::matrix(matrix &copy) 
{
    rows=copy.rows;
    columns=copy.columns;
    precision=copy.precision;
    array = new double [rows*columns];
    for( int i =0; i < rows * columns; ++i)
        array[i]=copy.array[i];
}
//деструктор
matrix::~matrix() 
{
    delete [] array;
}
//ввод данных из массива
matrix& matrix::set(const double *array) 
{
    for (int i = 0; i < rows * columns; i++) this->array[i] = array[i];
    return *this;
}
//моздание единичной матрицы
matrix& matrix::eye() 
{
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            array[j * columns + i] = i == j ? 1.0 : 0.0;
    return *this;
}
//создание нулевой матрицы
matrix& matrix::zero() 
{
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            array[j * columns + i] = 0.0;
    return *this;
}
//транспонированная матрица 
matrix matrix::transpose() 
{
    matrix temp(columns, rows);
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            temp.array[i * rows + j] = array[j * columns + i];
    return temp;
}
//оператор умножения матриц
matrix matrix::operator*(const matrix& right_arg) 
{
    matrix temp(rows, right_arg.columns);
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < right_arg.columns; i++) {
            temp.array[j * right_arg.columns + i] = 0.0;
            for (int k = 0; k < columns; k++)
                temp.array[j * right_arg.columns + i] += array[j * columns + k] * right_arg.array[k * right_arg.columns + i];
        }
    return temp;
}
//оператор умножения матрицы на число
matrix matrix::operator*(const double &s) 
{
    matrix temp(rows, columns);
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            temp.array[j * columns + i] = array[j * columns + i] * s;
    return temp;
}
//оператор сложения матриц
matrix matrix::operator+(const matrix& right_arg) 
{
    matrix temp(rows, columns);
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            temp.array[j * columns + i] = array[j * columns + i] + right_arg.array[j * columns + i];
    return temp;
}
//оператор вычитания матриц
matrix matrix::operator-(const matrix& right_arg) 
{
    matrix temp(rows, columns);
    for (int j = 0; j < rows; j++)
        for (int i = 0; i < columns; i++)
            temp.array[j * columns + i] = array[j * columns + i] - right_arg.array[j * columns + i];
    return temp;
}
//оператор присваивания
matrix& matrix::operator=(const matrix& right_arg) 
{
    if (rows * columns != right_arg.rows * right_arg.columns) {
        delete [] array;
        array = new double[right_arg.rows * right_arg.columns];
    }
    rows = right_arg.rows;
    columns = right_arg.columns;
 
    for (int i = 0; i < rows * columns; i++) array[i] = right_arg.array[i];
    return *this;
}
//оператор возвращения элемента
double& matrix::operator()(const int& i, const int& j)
{
    return (array[i*columns+j]);
}
//преобразование Хаусхолдера
void matrix::householderDecomposition(matrix& Q, matrix& R) 
{
    double mag, alpha;
    matrix x(rows, 1), u(rows, 1);
    matrix P(rows, rows), E(rows, rows);
 
    Q = matrix(rows, rows);
    R = *this;
 
    for (int i = 0; i < columns; i++) {
        x.zero(); u.zero();
        
        mag = 0.0;
        for (int j = i; j < rows; j++) {
            x.array[j] = R.array[j * columns + i];
            mag += x.array[j] * x.array[j];
        }
        mag = sqrt(mag);
        alpha = x.array[i] > 0 ? mag : -mag;
 
        mag = 0.0;
        for (int j = i; j < rows; j++) {
            u.array[j] = j == i ? x.array[j] + alpha : x.array[j];
            mag += u.array[j] * u.array[j];
        }
        mag = sqrt(mag);
 
        for (int j = i; j < rows; j++) u.array[j] /= mag;
 
        P = E - (u * u.transpose()) * 2.0;
 
        R = P * R;
        Q = Q * P;
    }
}
//вывод матрицы на экран
void matrix::output() 
{
    for (int j = 0; j < rows; j++) {
        for (int i = 0; i < columns; i++) {
            std::cout << std::fixed << std::setprecision(precision) << 
                std::setw(16) << array[j * columns + i] << "   ";
        }
        std::cout << std::endl;
    }
    std::cout << std::endl;
}
//вывод матрицы в файл
void matrix::output(std::ofstream* file)
{
    for (int j = 0; j < rows; j++) {
        for (int i = 0; i < columns; i++) {
            *file << std::fixed << std::setprecision(precision) << 
                std::setw(16) << array[j * columns + i] << "   ";
        }
        *file << std::endl;
    }
    *file << std::endl;
}
//изменение размеров матрицы
void matrix::SetSize (const int& rows, const int& columns)
{
    if (array!=NULL)
        delete[] array;
    array=new double[rows * columns];
    this->zero();
}
//возвращаение количества стобцов
int& matrix::Columns (void)
{
    return columns;
}
//возвращает количество строк
int& matrix::Rows (void)
{
    return rows;
}
//установка точности вывода
void matrix::SetPrecison(const int& precision)
{
    matrix::precision=precision;
}
Спасибо!
1
Лучшие ответы (1)
Надоела реклама? Зарегистрируйтесь и она исчезнет полностью.
Similar
Эксперт
41792 / 34177 / 6122
Регистрация: 12.04.2006
Сообщений: 57,940
25.04.2013, 23:39
Здравствуйте! Я подобрал для вас темы с ответами на вопрос Критику в студию (C++):

из Борланда в Студию - C++
уважаемые господа джуниоры и синьоры программисты. помогите мне чайнику разобраться с проблемой плиз есть проект (игра) написанная...

Как войти в Visual студию? - C++
Здравствуйте!Я те то чтобы начинающий,а ещё хуже!Начал изучать Либерти Джесс Освой С++ за 21 день,и сами знаете во всех книгах написание...

Где взять STLPort под 13 студию? - C++
Сабж. З.Ы В гугле был.

Как студию подружить со статической библиотекой скопилированой в MinGW ? - C++
Со скопилироваными в Visual Studio либами всё вроде норм, а с этим что-то артачится и выдаёт что не может найти определения функций: ...

Тестовое задание от работадателя. Хотелось бы услышать критику. - C++
Пробую устроиться программистом. Вот одна из компаний выслала мне тестовое задание следующего содержания: Немного поломав голову,...

Критику.Рекомендацию.Недочеты - Рабочая станция
Здравствуйте, вот собрал сборку. Хочу критику. Видео-Gigabite Radeon R9 280 ЖД-WD Blue 500Gb Процессор-Amd FX 8320 Кулер-Zalman...

30
ninja2
231 / 187 / 7
Регистрация: 26.09.2012
Сообщений: 2,018
Завершенные тесты: 1
25.04.2013, 23:54 #2
metaluga145, Красавец! Мне не чего добавить. Все правильно сделал.
Я могу только мелочь подсказать от ты когда создаешь заголовочный файл нужно обязательно, то что в нем записано записывать между #ifndef #define и #endif , это что бы случайно повторного включения не было.
По коду конечно притензий нету. От еще Страуструп советует типо в классе при перегрузке операторов которые не изменяют как бы класс ну от у тебя + - * и.т.п. не изменяют собственный класс, как бы их нужно определять глобально, а отакие +=, *= и.т.п. как бы членами класса делать. Ну это он так как бы советует делать. Ну это не сильно важно.
Вообщем мне понравилось молодчага.
0
BumerangSP
26.04.2013, 00:27
  #3

Не по теме:

Цитата Сообщение от ninja2 Посмотреть сообщение
#define и #endif , это что бы случайно повторного включения не было.
у него вместо этого стоит #pragma once.

0
alsav22
5426 / 4821 / 442
Регистрация: 04.06.2011
Сообщений: 13,587
26.04.2013, 00:27 #4
Два одинаковых кода: вывод матрицы на экран и вывод в файл, напрашивается заменить одним. В зависимости от потока, будет вывод или на экран, или в файл.
0
Olivеr
412 / 408 / 13
Регистрация: 06.10.2011
Сообщений: 832
26.04.2013, 01:35 #5
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
2
3
4
5
//возвращаение количества стобцов
int& matrix::Columns (void)
{
* * return columns;
}
Вы возвращаете ссылку, тогда я в любое время в любом месте могу написать
C++
1
2
matrix arr(5,5);
arr.Columns() = 4; //ok: теперь arr.columns равно 4
Если хотите избежать этого, то возвращайте копию или константную ссылку или сделайте this константным.

Добавлено через 1 минуту
Цитата Сообщение от ninja2 Посмотреть сообщение
По коду конечно притензий нету. От еще Страуструп советует типо в классе при перегрузке операторов которые не изменяют как бы класс ну от у тебя + - * и.т.п. не изменяют собственный класс, как бы их нужно определять глобально, а отакие +=, *= и.т.п. как бы членами класса делать. Ну это он так как бы советует делать. Ну это не сильно важно.
имеется, наверное, перегрузка операторов с использованием дружественных функций...

И для чего Вам нужен виртуальный деструктор?
1
MrGluck
Модератор
Эксперт CЭксперт С++
7492 / 4607 / 693
Регистрация: 29.11.2010
Сообщений: 12,605
26.04.2013, 03:36 #6
Не увидел оператора присваиваний.
В нескольких местах не помешало бы поставить Enter, хоть скроллбар убрать (я вообще считаю, что код должен иметь ширину не более 80 символов)

Цитата Сообщение от metaluga145 Посмотреть сообщение
//моздание единичной матрицы
no comments

Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
array[j * columns + i] = i == j ? 1.0 : 0.0;
C++
1
array[j * columns + i] = (i == j);
//создание нулевой матрицы
C++
1
array = {0};
Непонятно, зачем требуется формировать объект и далее возвращать ссылку на него, а не просто работать внутри объекта же.

Комменты желательно подтянуть на один уровень отступов, просто для ++читаемость.

Сделать конст методы там, где нужно.

Это по заданию нужно извращаться с хранением матрицы внутри одномерного массива?

Цитата Сообщение от Olivеr Посмотреть сообщение
И для чего Вам нужен виртуальный деструктор?
Возможно, он думает, что от его супер-класса будут наследоваться.
0
0x10
2480 / 1655 / 248
Регистрация: 24.11.2012
Сообщений: 4,102
26.04.2013, 04:47 #7
1. Класс так и просится стать шаблонным.
2. Не вижу необходимости использовать сишные массивы, можно спокойно использовать std::vector.
3.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
matrix(const int& , const int& , const double *array);//конструктор, берет значения из массива
Вот через этот конструктор наружу торчат детали реализации матрицы - видно, что внутри нее одномерный массив. Такая реализация вполне ожидаема, но не хочется наблюдать ее в явном виде. Плюс - зачем значения типа int передавать по ссылке?
4.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
matrix::matrix(matrix &);//конструктор копирования
const
5. Было бы неплохо добавить move-конструктор.
6.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
int precision;//точность вывода матрицы
Т.е. до этого говорили о матрицах, а тут внезапно заговорили о точности вывода. Этому полю в классе делать явно нечего, поскольку к данным отношения никакого не имеет.
7.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
matrix& set(const double* );//функция изминения значения всех элементов матрицы
И нигде нет проверки на выход за границы передаваемого массива. Лучше этот метод убрать, поскольку есть метод, предоставляющий доступ к элементам матрицы - с его помощью можно устанавливать значения снаружи.
8.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
matrix& eye();//делает матрицу еденичной(возможно не квадратной)
Зачем? Часто приходится выполнять такую операцию? Опять же - это можно сделать и внешними средствами.
9. Метод zero туда же.
10. Операторы посмотрел очень бегло. То, что должны быть константными - уже сказали. Сходу не увидел проверок на выход за границы массивов.
11.
Цитата Сообщение от metaluga145 Посмотреть сообщение
C++
1
void householderDecomposition(matrix& Q, matrix& R);//алгоритм Хаусхолдера, изменяет параметры-ссылки
Зачем этот метод в классе? Сделать обычной функцией.
12. Методы вывода также вынести из класса.
13. "SetPrecison" также не нужен в классе.
2
Croessmah
Ушел
Эксперт CЭксперт С++
13554 / 7705 / 872
Регистрация: 27.09.2012
Сообщений: 19,006
Записей в блоге: 3
Завершенные тесты: 1
26.04.2013, 10:09 #8
имхо, класс не должен содержать методов вывода. Лучше уж перегрузить <<. Да и сам класс лучше сделать шаблонным.

C++
1
int& matrix::Columns
Про ссылку уже написали, добавлю только то, что если уж используете const, то используйте по полной. Данный метод не меняет состояние объекта, так что делаем его const. Тоже самое относится и к другим методам, не меняющих объект.

Добавлено через 2 минуты
Так же, если есть операторы "+", "-" и другие, то сделайте и оператор "+="

Добавлено через 1 минуту
C++
1
2
3
4
matrix::matrix():rows(0),columns(0),precision(2)
{
    array=NULL;
}
не лучше ли array тоже инициализировать в списке инициализации?

Добавлено через 43 секунды
Так же не видно индексации

Добавлено через 2 минуты
C++
1
2
3
4
5
6
7
void matrix::SetSize (const int& rows, const int& columns)
{
    if (array!=NULL)
        delete[] array;
    array=new double[rows * columns];
    this->zero();
}
Старую память удалили, новый кусок взяли, а новые размеры установить в классе?
+ может мне не нужно её обнулять?
+ может мне нужна матрица такого же размера или меньше?
0
Tulosba
:)
Эксперт С++
4397 / 3233 / 297
Регистрация: 19.02.2013
Сообщений: 9,045
26.04.2013, 12:40 #9
При умножении/сложении/вычитании матриц нет проверки на согласованность размеров матриц.
Поля, объявленные protected надо бы сделать private.
Потроха сделать на базе std::vector. А еще непонятно зачем имитация двумерности на базе одномерного массива.
0
diagon
Higher
1930 / 1196 / 49
Регистрация: 02.05.2010
Сообщений: 2,925
Записей в блоге: 2
26.04.2013, 13:17 #10
Цитата Сообщение от Tulosba Посмотреть сообщение
А еще непонятно зачем имитация двумерности на базе одномерного массива.
Так быстрее. Ну, выделение памяти чуть дольше (и то не всегда), зато доступ к такой матрице значительно эффективнее.
Цитата Сообщение от Tulosba Посмотреть сообщение
Потроха сделать на базе std::vector
А еще лучше через std::valarray. В студии эта штука, правда, тормозит сильнее вектора, зато в gcc она очень мощно оптимизирована. Ну и предлагает свой няшный сахарок в виде перегруженных операторов сложения и прочего.
0
Kastaneda
Jesus loves me
Эксперт С++
4689 / 2893 / 236
Регистрация: 12.12.2009
Сообщений: 7,357
Записей в блоге: 2
Завершенные тесты: 1
26.04.2013, 13:18 #11
Передавать POD типы по const ссылке незачем, передавай по значению.
Интерфейс класса
C++
1
2
3
4
5
public:
...
protected:
...
public:
зачем это? По хорошему я даже не должен дойти глазами до protected или private, меня, как пользователя класса, интересует то, что public.
1
Tulosba
:)
Эксперт С++
4397 / 3233 / 297
Регистрация: 19.02.2013
Сообщений: 9,045
26.04.2013, 14:03 #12
Цитата Сообщение от diagon Посмотреть сообщение
зато доступ к такой матрице значительно эффективнее.
То, что все элементы будут лежать рядом в одномерном массиве, еще не значит, что в двумерном не будет нечто подобного. Но при этом в коде выглядит всё более очевиднее. Откуда тут следует "значительно эффективнее"? К тому же, не стоит забывать тезис про "преждевременную оптимизацию".

Добавлено через 1 минуту
Цитата Сообщение от Kastaneda Посмотреть сообщение
Передавать POD типы по const ссылке незачем, передавай по значению.
А если POD-структура большая, зачем копирование?
0
Kastaneda
Jesus loves me
Эксперт С++
4689 / 2893 / 236
Регистрация: 12.12.2009
Сообщений: 7,357
Записей в блоге: 2
Завершенные тесты: 1
26.04.2013, 14:18 #13
Цитата Сообщение от Tulosba Посмотреть сообщение
А если POD-структура большая, зачем копирование?
я имел ввиду т.н. "плоский тип", т.е. char, short, int и т.д.
0
Tulosba
:)
Эксперт С++
4397 / 3233 / 297
Регистрация: 19.02.2013
Сообщений: 9,045
26.04.2013, 14:23 #14
Цитата Сообщение от Kastaneda Посмотреть сообщение
я имел ввиду т.н. "плоский тип", т.е. char, short, int и т.д.
по-моему, это называется "фундаментальный тип"
0
ForEveR
В астрале
Эксперт С++
7983 / 4742 / 321
Регистрация: 24.06.2010
Сообщений: 10,545
Завершенные тесты: 3
26.04.2013, 14:25 #15
Tulosba, В данном случае уместнее будет сказать скалярные.
Arithmetic types (3.9.1), enumeration types, pointer types, pointer to member types (3.9.2), std::nullptr_-
t, and cv-qualified versions of these types (3.9.3) are collectively called scalar types
1
26.04.2013, 14:25
MoreAnswers
Эксперт
37091 / 29110 / 5898
Регистрация: 17.06.2006
Сообщений: 43,301
26.04.2013, 14:25
Привет! Вот еще темы с ответами:

Прошу критику курсов - Java
Я начинающий, образование экономическое. Хочу пойти сюда: http://mti.edu.ru/entrance/retraining/it/dev/javawebdev Стоит?

Можно услышать критику - HTML, CSS
Я верстал шапку сайта, это моя первая такая работа. http://white.net76.net/ Покритикуйте, пожалуйста

хочу критику сайта объективную - HTML, CSS
сделал свой первый сайт и хотелось бы услышать мнение опытных: что поменять, как оптимизировать и т. д. и т. п. В общем поделитесь опытом...

С++ Builder XE 5 включить критику синтаксиса - C++ Builder
Возможно ли в С++ Builder XE 5 включить критику синтаксиса на подобии visual studio 2012?


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

Или воспользуйтесь поиском по форуму:
15
Ответ Создать тему
Опции темы

КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin® Version 3.8.9
Copyright ©2000 - 2017, vBulletin Solutions, Inc.
Рейтинг@Mail.ru