244 / 245 / 38
Регистрация: 08.04.2013
Сообщений: 927
|
|||||||||||
1 | |||||||||||
Критику в студию25.04.2013, 23:39. Показов 5605. Ответов 30
Метки нет (Все метки)
Доброго всем времени суток! Я вот написал такой себе класс матриц, прошу оценить его и посоветовать что можно улучшить или изменить, как повысить быстродействие или, возможно, что-то надо добавить.
Знаю,что надо сделать шаблон класса для разных типов данных, но дабы сейчас все было красиво и хорошо не загрязнял этим код! Любые дельные советы будут "лайкнуты"! matrix.h
1
|
25.04.2013, 23:39 | |
Ответы с готовыми решениями:
30
из Борланда в Студию Тестовое задание от работадателя. Хотелось бы услышать критику. Как войти в Visual студию? Где взять STLPort под 13 студию? |
979 / 196 / 33
Регистрация: 26.09.2012
Сообщений: 2,041
|
|
25.04.2013, 23:54 | 2 |
metaluga145, Красавец! Мне нечего добавить. Все правильно сделал.
Я могу только мелочь подсказать от ты когда создаешь заголовочный файл нужно обязательно, то что в нем записано записывать между #ifndef #define и #endif , это что бы случайно повторного включения не было. По коду конечно притензий нету. От еще Страуструп советует типо в классе при перегрузке операторов которые не изменяют как бы класс ну от у тебя + - * и.т.п. не изменяют собственный класс, как бы их нужно определять глобально, а отакие +=, *= и.т.п. как бы членами класса делать. Ну это он так как бы советует делать. Ну это не сильно важно. Вообщем мне понравилось молодчага.
0
|
BumerangSP
|
26.04.2013, 00:27
#3
|
0
|
415 / 411 / 95
Регистрация: 06.10.2011
Сообщений: 832
|
||||||
26.04.2013, 01:35 | 5 | |||||
Вы возвращаете ссылку, тогда я в любое время в любом месте могу написать
Добавлено через 1 минуту имеется, наверное, перегрузка операторов с использованием дружественных функций... И для чего Вам нужен виртуальный деструктор?
1
|
Форумчанин
8215 / 5045 / 1437
Регистрация: 29.11.2010
Сообщений: 13,453
|
|||||||||||
26.04.2013, 03:36 | 6 | ||||||||||
Не увидел оператора присваиваний.
В нескольких местах не помешало бы поставить Enter, хоть скроллбар убрать (я вообще считаю, что код должен иметь ширину не более 80 символов) no comments
Комменты желательно подтянуть на один уровень отступов, просто для ++читаемость. Сделать конст методы там, где нужно. Это по заданию нужно извращаться с хранением матрицы внутри одномерного массива? Возможно, он думает, что от его супер-класса будут наследоваться.
0
|
3257 / 2059 / 351
Регистрация: 24.11.2012
Сообщений: 4,909
|
|
26.04.2013, 04:47 | 7 |
1. Класс так и просится стать шаблонным.
2. Не вижу необходимости использовать сишные массивы, можно спокойно использовать std::vector. 3. Вот через этот конструктор наружу торчат детали реализации матрицы - видно, что внутри нее одномерный массив. Такая реализация вполне ожидаема, но не хочется наблюдать ее в явном виде. Плюс - зачем значения типа int передавать по ссылке? 4. const 5. Было бы неплохо добавить move-конструктор. 6. Т.е. до этого говорили о матрицах, а тут внезапно заговорили о точности вывода. Этому полю в классе делать явно нечего, поскольку к данным отношения никакого не имеет. 7. И нигде нет проверки на выход за границы передаваемого массива. Лучше этот метод убрать, поскольку есть метод, предоставляющий доступ к элементам матрицы - с его помощью можно устанавливать значения снаружи. 8. Зачем? Часто приходится выполнять такую операцию? Опять же - это можно сделать и внешними средствами. 9. Метод zero туда же. 10. Операторы посмотрел очень бегло. То, что должны быть константными - уже сказали. Сходу не увидел проверок на выход за границы массивов. 11. Зачем этот метод в классе? Сделать обычной функцией. 12. Методы вывода также вынести из класса. 13. "SetPrecison" также не нужен в классе.
2
|
Неэпический
|
||||||||||||||||
26.04.2013, 10:09 | 8 | |||||||||||||||
имхо, класс не должен содержать методов вывода. Лучше уж перегрузить <<. Да и сам класс лучше сделать шаблонным.
Добавлено через 2 минуты Так же, если есть операторы "+", "-" и другие, то сделайте и оператор "+=" Добавлено через 1 минуту
Добавлено через 43 секунды Так же не видно индексации Добавлено через 2 минуты
+ может мне не нужно её обнулять? + может мне нужна матрица такого же размера или меньше?
0
|
:)
4773 / 3267 / 497
Регистрация: 19.02.2013
Сообщений: 9,046
|
|
26.04.2013, 12:40 | 9 |
При умножении/сложении/вычитании матриц нет проверки на согласованность размеров матриц.
Поля, объявленные protected надо бы сделать private. Потроха сделать на базе std::vector. А еще непонятно зачем имитация двумерности на базе одномерного массива.
0
|
Higher
|
|
26.04.2013, 13:17 | 10 |
Так быстрее. Ну, выделение памяти чуть дольше (и то не всегда), зато доступ к такой матрице значительно эффективнее.
А еще лучше через std::valarray. В студии эта штука, правда, тормозит сильнее вектора, зато в gcc она очень мощно оптимизирована. Ну и предлагает свой няшный сахарок в виде перегруженных операторов сложения и прочего.
0
|
26.04.2013, 13:18 | 11 | |||||
Передавать POD типы по const ссылке незачем, передавай по значению.
Интерфейс класса
1
|
:)
4773 / 3267 / 497
Регистрация: 19.02.2013
Сообщений: 9,046
|
|
26.04.2013, 14:03 | 12 |
То, что все элементы будут лежать рядом в одномерном массиве, еще не значит, что в двумерном не будет нечто подобного. Но при этом в коде выглядит всё более очевиднее. Откуда тут следует "значительно эффективнее"? К тому же, не стоит забывать тезис про "преждевременную оптимизацию".
Добавлено через 1 минуту А если POD-структура большая, зачем копирование?
0
|
В астрале
8049 / 4806 / 655
Регистрация: 24.06.2010
Сообщений: 10,562
|
|
26.04.2013, 14:25 | 15 |
Tulosba, В данном случае уместнее будет сказать скалярные.
1
|
Kastaneda
|
26.04.2013, 14:38
#16
|
Не по теме: Да, точно, заглянул в стандарт, к POD еще структуры, юнионы и массивы скалярных типов относятся.
0
|
MrGluck
|
26.04.2013, 14:56
#18
|
Не по теме: Kastaneda, и тривиальные классы, если говорить о С++11
0
|
Higher
|
|
26.04.2013, 15:03 | 19 |
Сообщение было отмечено как решение
Решение
Что-то я не слышал о компайлерских оптимизациях "превращение массива указателей в одномерный массив".
Для доступа к элементу требуется одно разыменование, а не два, как в случае с массивом указателей. Ну, это копеечная оптимизация, это да. А вот кеш для линейного массива работает значительно лучше. Учитывая то, что доступ к памяти - наиболее времязатратная операция, получаем значительный выигрыш. Ну и еще векторизация и префетчинг для линейного массива делается гораздо проще, чем для массива указателей. А это тоже достаточно неслабое ускорение. В качестве бонуса, менеджер памяти гораздо быстрее поймет, что данная матрица является одним большим объектом, а не кучкой мелких. Это благотворно повлияет на общий перформанс. Ну и еще потребуется чуть-чуть меньше памяти (не понадобится хранить массив указателей). Итого: получаем более бережное обращение с памятью и кешем в частности, что может дать очень даже неслабое ускорение (в некоторых случаях - в десятки раз).
5
|
26.04.2013, 15:05 | 20 |
metaluga145, а где в классе перегруженые операторы [][]? Это же важная штука для матрицы.
0
|
26.04.2013, 15:05 | |
26.04.2013, 15:05 | |
Помогаю со студенческими работами здесь
20
Как студию подружить со статической библиотекой скопилированой в MinGW ? На критику Надеюсь на критику Можно услышать критику Искать еще темы с ответами Или воспользуйтесь поиском по форуму: |