История изменений
Исправление DarkEld3r, (текущая версия) :
Вникать лень, так что просто покритикую код, можешь считать придирками и игнорировать, если хочешь:
1. Закомментированный код - отстой. Да и разве не будет предупреждения на неиспользуемую функцию testSpeed? Кстати, тесты завести было бы неплохо.
2. Хорошая практика, делать конструкторы с одним параметром explicit (если нет оснований делать иначе).
3. Неплохо бы добавить move конструктор и оператор присваивания.
4. Зачем в заголовочном файле m_whitespaces, m_operators, m_functions? Я бы перенёс в cpp (в безымянный неймспейс).
5. throw() надо заменить на noexcep.
6. Использовать override.
7. Про смартпоинтеры уже сказали.
8. Вывод посреди логики, я бы как-то разделить постарался.
9. return (m_root == e.m_root) || (*m_root == *e.m_root); Это что? Один из указателей не может оказаться nullptr? Если может, то будет плохо.
10. «Инклюд гарды», конечно, дело вкуса, но pragma once понимают все современные компиляторы.
11. Местами include тебя странно сгруппированы. В основном, iostream особняком стоит, но в expression.cpp - нет.
12. Авто, вроде, используешь, но не везде: Functions<int>::const_iterator res = m_operators.end();
Исправление DarkEld3r, :
Вникать лень, так что просто покритикую код, можешь считать придирками и игнорировать, если хочешь:
1. Закомментированный код - отстой. Да и разве не будет предупреждения на неиспользуемую функцию testSpeed? Кстати, тесты завести было бы неплохо.
2. Хорошая практика, делать конструкторы с одним параметром explicit (если нет оснований делать иначе).
3. Неплохо бы добавить move конструктор и оператор присваивания.
4. Зачем в заголовочном файле m_whitespaces, m_operators, m_functions? Я бы перенёс в cpp (в безымянный неймспейс). 5. throw() надо заменить на noexcep. 6. Использовать override. 7. Про смартпоинтеры уже сказали. 8. Вывод посреди логики, я бы как-то разделить постарался. 9. return (m_root == e.m_root) || (*m_root == *e.m_root); Это что? Один из указателей не может оказаться nullptr? Если может, то будет плохо. 10. «Инклюд гарды», конечно, дело вкуса, но pragma once понимают все современные компиляторы. 11. Местами include тебя странно сгруппированы. В основном, iostream особняком стоит, но в expression.cpp - нет. 12. Авто, вроде, использует, но не везде: Functions<int>::const_iterator res = m_operators.end();
Исходная версия DarkEld3r, :
Вникать лень, так что просто покритикую код, можешь считать придирками и игнорировать, если хочешь: 1. Закомментированный код - отстой. Да и разве не будет предупреждения на неиспользуемую функцию testSpeed? Кстати, тесты завести было бы неплохо. 2. Хорошая практика, делать конструкторы с одним параметром explicit (если нет оснований делать иначе). 3. Неплохо бы добавить move конструктор и оператор присваивания. 4. Зачем в заголовочном файле m_whitespaces, m_operators, m_functions? Я бы перенёс в cpp (в безымянный неймспейс). 5. throw() надо заменить на noexcep. 6. Использовать override. 7. Про смартпоинтеры уже сказали. 8. Вывод посреди логики, я бы как-то разделить постарался. 9. return (m_root == e.m_root) || (*m_root == *e.m_root); Это что? Один из указателей не может оказаться nullptr? Если может, то будет плохо. 10. «Инклюд гарды», конечно, дело вкуса, но pragma once понимают все современные компиляторы. 11. Местами include тебя странно сгруппированы. В основном, iostream особняком стоит, но в expression.cpp - нет. 12. Авто, вроде, использует, но не везде: Functions<int>::const_iterator res = m_operators.end();