refactor: Centralize error output #3902
Conversation
… link between GEOS_THROW_CTX_IF and LVARRAY_THROW_IF_TEST( EXP, MSG, TYPE )
… in try/catch statements Problem: Retrieves everything that was thrown, so not just the message.
…/catch in main)": remove useless try/catch
…y spaces. The previous condition checked whether an argument was present and whether the option was immediately followed by a value like -test"value", which excluded valid cases like -test "value" et -test "value".
| /// The signal received and formatted | ||
| std::string m_signal; |
There was a problem hiding this comment.
That little guy seems to never be initialized (addSignalToMsg()?)
Maybe you can just remove it and read the Attribute::Signal entry
| * @param errMsg Class containing all the error/warning information | ||
| * @param oss The output stream to write the content to. | ||
| */ | ||
| void ErrorLogger::writeToAscii( ErrorLogger::ErrorMsg const & errMsg, std::ostream & oss ) |
There was a problem hiding this comment.
maybe move that just over writeToYaml?
| /// String containing the target object name followed by the the file and line declaring it. | ||
| string m_dataDisplayString; |
There was a problem hiding this comment.
As the auto-constructor is extensively used in your PR, I would suggest moving m_dataDisplayString at top of m_attributes for clarity and to force to give this parameters.
| oss << "***** LOCATION: " << errMsg.m_file<< " l." << errMsg.m_line << "\n"; | ||
| oss << "***** " << errMsg.m_cause << "\n"; |
There was a problem hiding this comment.
Be careful, it seems like ErrorMsg can be constructed without those. You can output them conditionnally or remove the empty ErrorMsg constructor (there are also solutions between the two).
| /** | ||
| * @brief Format all information in ErrorMsg and write it to the specified output stream | ||
| * @param errMsg The struct containing the error/warning object | ||
| * @param oss The output stream | ||
| */ | ||
| static void writeToAscii( ErrorLogger::ErrorMsg const & errMsg, std::ostream & oss ); | ||
|
|
There was a problem hiding this comment.
Same remark, can you move it with the associated functions?
Maybe I see why you ordered it that way, yes it is static, but it is a function, so you can place it wherever you want.
If you prefer it as it is, don't hesitate to tell.
src/main/main.cpp
Outdated
| } | ||
| catch( std::exception const & e ) | ||
| { // native exceptions management | ||
| ErrorLogger::ErrorMsg & errMsg = ErrorLogger::global().currentErrorMsg(); |
There was a problem hiding this comment.
I think it lacks a lot of information here: at this point, IIRC, GEOS_THOW() has never been called, so the same information must be provided to the error message.
Appart from that, this part is a lot better!
Remove code duplication found in
GEOS_THROW,GEOS_ERROR,GEOS_WARNINGand put into a static function inErrorLogger.Called while
flushErrorMsg().Move all Exceptions under GeosExceptions.hpp