Во первых, хорошо, что вы потратили время и разобрались с boost::regex, boost::shared_ptr, и фреймворком для тестирования. Сейчас ваша работа гораздо более похожа на продукт.
Однако замечания все еще есть:
- Автоматические тесты должны тестировать не только функциональность но и поведение библиотеки при работе с некорректными или неожидаемыми данными. В вашем случае – это некорректный ini файл на входе.
- Более того в вашем коде совершенно не видно никакой стратегии обработки ошибок (даже в стиле C). Я попытался прочесть из строкового аргумента число при помощи getIntParamValue и получил исключение из boost::lexical_cast. Очевидно это не самое лучшее поведение с точки зрения пользователя.
- На мой взгляд следующий код пользователя излишне многословен:
Я думаю можно предоставить пользователю гораздо более удобный интерфейс, например:
getMandatoryParam должна выкидывать исключение (ведь пользователь может потребовать обязательного наличия некоторых полей), по которому пользователь сможет точно восстановить причину ошибки. getOptionalParam подставит defaultAudioDevice в audioDevice если параметра в тэге нет. Такие же функции можно определить для getInt, getBool, etc.
Кстати, this-> совершенно излишне и в данной ситуации только путает читателя.
- Я бы еще посоветовал выделить Tag как отдельную сущность, с ConfigData было бы удобнее работать. (см. например паттерн Proxy).
- Последнее. Вы наверное пропустили, в моем предыдущем письме:
Использовать std::string* менее безопасно чем std::string&. Предпочитайте использовать неконстантные ссылки для аргументов, которые предполагают изменение передаваемого параметра:
