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

C++

Войти
Регистрация
Восстановить пароль
 
dvano
131 / 62 / 16
Регистрация: 18.06.2014
Сообщений: 216
Завершенные тесты: 1
#1

Оценить код - C++

01.04.2016, 23:06. Просмотров 376. Ответов 5
Метки нет (Все метки)

Добрый день. В данный момент пишу Ray tracing. Если у вас есть время, оцените код. Какие советы вы можете дать? Что можно улучшить(исправить)? Как бы вы сделали ту или иную вещь?
[cut]
Similar
Эксперт
41792 / 34177 / 6122
Регистрация: 12.04.2006
Сообщений: 57,940
01.04.2016, 23:06     Оценить код
Посмотрите здесь:

C++ Прошу оценить свой уровень
Оценить корретность программы. Visual C++
C++ Просьба оценить код
C++ Оценить правильность использования конструкторов, деструкторов в коде
C++ WinAPI Как оценить ширину текста, выводимого функцией TextOut?
Прошу оценить готовую программу Visual C++
Нужно оценить время запуска программы C++
Оценить время выполнения и сложность простейших операций с разными типами данных C++
C++ Как оценить объём кучи
Калькулятор для начинающих, прошу оценить C++
C++ Вычислить произведение рекурсивно/итеративно, оценить время выполнения
Оценить справедливость разложения заданной функции в ряд Тейлора C++

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

Или воспользуйтесь поиском по форуму:
После регистрации реклама в сообщениях будет скрыта и будут доступны все возможности форума.
ct0r
Игогошка!
1762 / 664 / 42
Регистрация: 19.08.2012
Сообщений: 1,261
Завершенные тесты: 1
02.04.2016, 12:44     Оценить код #2
Где тесты, документация?
Зачем пихать во все места noexcept, final, pimpl?
dvano
131 / 62 / 16
Регистрация: 18.06.2014
Сообщений: 216
Завершенные тесты: 1
02.04.2016, 12:54  [ТС]     Оценить код #3
Цитата Сообщение от ct0r Посмотреть сообщение
Зачем пихать во все места noexcept, final, pimpl?
А почему бы не впихнуть noexcept туда, где исключения отсутствуют? И к тому же уменьшает количество кода, который генерирует компилятор. Там где не нужно наследование - final. Pimpl позволяет скрыть весь мусор в cpp.
ct0r
Игогошка!
1762 / 664 / 42
Регистрация: 19.08.2012
Сообщений: 1,261
Завершенные тесты: 1
02.04.2016, 13:33     Оценить код #4
Цитата Сообщение от dvano Посмотреть сообщение
А почему бы не впихнуть noexcept туда, где исключения отсутствуют?
Они не просто должны там отсутствовать сейчас. Ты должен понимать, что они будут там отсутствовать и в будущем. Иначе будут проблемы с сопровождением.

Цитата Сообщение от dvano Посмотреть сообщение
И к тому же уменьшает количество кода, который генерирует компилятор.
Конкретно у тебя какой от этого профит? Ты замерял? Он важен?

Цитата Сообщение от dvano Посмотреть сообщение
Там где не нужно наследование - final.
А тут в чем профит? Показать, что класс не будет базовым? Ну спасибо, очень ценная инфа. final не для этого был придуман.

Цитата Сообщение от dvano Посмотреть сообщение
Pimpl позволяет скрыть весь мусор в cpp.
Если у тебя много мусора в cpp, то нужно бороться с причиной, а не прятать последствия. pimpl используется для предоставления API с устойчивым ABI, а также для сокращения времени компиляции. Пока не вижу у тебя таких нужд.
hoggy
6168 / 2534 / 444
Регистрация: 15.11.2014
Сообщений: 5,612
Завершенные тесты: 1
02.04.2016, 16:22     Оценить код #5
Цитата Сообщение от dvano Посмотреть сообщение
оцените код
в целом: код приятен.
с точки зрения чтения, и восприятия,
можно назвать образцово показательным.

Цитата Сообщение от dvano Посмотреть сообщение
Какие советы вы можете дать? Что можно улучшить(исправить)?
можно убить сразу же трех зайцев,
если добавить дополнительную папку,
которая содержала бы примеры использования.

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

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

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

Цитата Сообщение от dvano Посмотреть сообщение
Как бы вы сделали ту или иную вещь?
рассмотрим код:
C++
1
void insertObject(std::unique_ptr<RenderObject>&& renderObject) noexcept;
такие вещи на мой взгляд лучше прятать в приваты.
потому что std::unique_ptr в данном случае - это деталь реализации.

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

вместо этого:
C++
1
2
3
4
5
6
7
scene.insertObject(
    std::make_unique<Sphere>(
        vec3f{0.0F, 0.0F, 100.0F}, 
        vec3f{0.0F, 0.0F, 255.0F}, 
        50.0F
    )
);
вот такое:
C++
1
2
3
4
5
6
7
// укажите тип объекта, который нужно вставить
// и список аргументов с которыми он должен быть построен
scene.insertObject<Sphere>(
    vec3f{0.0F, 0.0F, 100.0F}, 
    vec3f{0.0F, 0.0F, 255.0F}, 
    50.0F
);
запись получается короче, что уже удобнее само по себе.
и не содержит намеков на детали реализации.

достичь этого можно за счет статической фабрики:

C++
1
2
3
4
5
6
7
8
template<class RenderObject_, class ...Args>
void insertObject(Args&&... args) noexcept
{
    // вызываем приватную деталь реализации
    this->insertObject(
        std::make_unique<RenderObject_>( std::forward<Args>(args)... )
    );
}
получаем тоже самое,
но пользователям больше не нужно знать о факте использования std::unique_ptr.
а вы соответственно, не имеете легаси в связи с этим.
ct0r
Игогошка!
1762 / 664 / 42
Регистрация: 19.08.2012
Сообщений: 1,261
Завершенные тесты: 1
02.04.2016, 17:35     Оценить код #6
Еще глянул.

1) Почти не видно DbC (Design-by-Contract) и DP (Defensive Programming).
2) move ctors и move assignment operators написаны совершенно бессмысленно.
3) Если уж делаем pimpl, то почему без поддержки перемещений? И зачем там еще в присваивании постоянно делать новый unique_ptr?

Остальное вроде окей.
Yandex
Объявления
02.04.2016, 17:35     Оценить код
Ответ Создать тему
Опции темы

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